Skip to content

[Bug]: Second process's close() discards the first process's committed writes (single-file storage, 0.5.42) #405

Description

@asm0dey

What happened?

Two processes that open the same on-disk database both succeed, and the one that
closes last overwrites the whole store with its own in-memory view. Writes the
other process already committed are gone. Neither process raises an error, and
neither execute() nor close() returns anything that indicates a conflict.

Expected: either the writes from both processes are retained, or the second
GrafeoDB(path) fails with a clear "already open" error.

Observed: the last writer to close silently wins.

This looks like the multi-process consequence of the design described in #327 —
single-file .grafeo storage checkpoints the in-memory LPG store into the
container at close(). With one process that is correct. With two, the second
checkpoint is built from a store that never saw the first process's commits, so
writing it discards them. #395 reports a related symptom of the same
close()-owns-persistence model from a single process.

The interleaving, with the barrier ordering from the reproduction below:

sequenceDiagram
    participant P1 as Process 1
    participant Disk as graph.db (single file)
    participant P2 as Process 2

    P1->>Disk: GrafeoDB(path)
    Note over P1: in-memory store = {}
    P2->>Disk: GrafeoDB(path) — succeeds, no error
    Note over P2: in-memory store = {}
    P1->>P1: INSERT A — commits, returns OK
    P1->>Disk: close() → snapshot {A}
    P2->>P2: INSERT B — commits, returns OK
    P2->>Disk: close() → snapshot {B}
    Note over Disk: file now holds {B}.<br/>A is gone: P2's snapshot was built<br/>from a store that never saw it.
Loading

Because the checkpoint writes the whole store rather than a delta, close() is
effectively "the file is now what I have in memory" rather than "add my work to
the file". With a single process that distinction never shows.

Two details that make this worse than a plain "don't do that":

  • It is silent. In a 4-writer race, all writers reported success and 100 of
    200 writes were missing. Callers have no signal to retry or fail on.
  • The Python binding cannot avoid it. grafeo.GrafeoDB exposes only
    GrafeoDB() and GrafeoDB(path); there is no way to select WalDirectory
    storage, and dir(grafeo) exports no storage-format or config type. Python
    users are on the affected path by default with no documented opt-out.

I could not find a statement in README.md, docs/, or CONTRIBUTING.md that an
on-disk database is single-process-only, so I am reporting it as a bug rather
than as a documentation gap. If single-process is the intended contract, a
loud failure at open plus a line in the docs would be enough to close this.

Either resolution would work for my use case:

  1. Make concurrent opens safe (shared WAL, or reload-before-checkpoint), or
  2. Fail the second open with an explicit error, the way SQLite and other
    embedded engines do, and document the constraint.

An error at open is the more valuable of the two, because silent loss is the
part that is hard to defend against from the outside.

AI Disclosure: This issue was prepared with the assistance of Claude
(Anthropic) running in Claude Code. The failure was found while I was
benchmarking Grafeo as an embedded store for a personal project; the AI
assistant ran the experiments, reduced the race to the deterministic barrier
version above, and drafted this report. I ran the reproduction myself and
verified the reported output before submitting. No fix or patch is proposed
here.

How to reproduce

Deterministic — reproduces 3 out of 3 runs. Two processes are interleaved with
file barriers so the ordering is fixed:

P1 open -> P2 open -> P1 INSERT A, close -> P2 INSERT B, close -> read back
# repro_deterministic.py   —   python repro_deterministic.py
import os, shutil, subprocess, sys, tempfile, time, pathlib
import grafeo

TMP = pathlib.Path(tempfile.gettempdir())
DB = TMP / "grafeo_det.db"

def wait_for(flag, timeout=30):
    t = time.time()
    while not (TMP / flag).exists():
        if time.time() - t > timeout:
            raise SystemExit(f"timeout waiting {flag}")
        time.sleep(0.02)

def touch(flag):
    (TMP / flag).write_text("1")

if len(sys.argv) > 1:
    role = sys.argv[1]
    if role == "p1":
        db = grafeo.GrafeoDB(str(DB)); touch("p1_open")
        wait_for("p2_open")
        db.execute("INSERT (:Item {id:'A'})")
        db.close(); touch("p1_done")
    else:
        wait_for("p1_open")
        db = grafeo.GrafeoDB(str(DB)); touch("p2_open")
        wait_for("p1_done")
        db.execute("INSERT (:Item {id:'B'})")
        db.close(); touch("p2_done")
    sys.exit(0)

for f in ("p1_open", "p2_open", "p1_done", "p2_done"):
    (TMP / f).unlink(missing_ok=True)
shutil.rmtree(DB, ignore_errors=True); DB.unlink(missing_ok=True)
kids = [subprocess.Popen([sys.executable, __file__, r]) for r in ("p1", "p2")]
codes = [k.wait() for k in kids]
db = grafeo.GrafeoDB(str(DB))
rows = sorted(r["id"] for r in db.execute("MATCH (n:Item) RETURN n.id AS id"))
print(f"exit {codes}  expected ['A','B']  got {rows}"
      f"  -> {'OK' if rows == ['A','B'] else 'DATA LOSS'}")

Output, three consecutive runs:

exit [0, 0]  expected ['A','B']  got ['B']  -> DATA LOSS
exit [0, 0]  expected ['A','B']  got ['B']  -> DATA LOSS
exit [0, 0]  expected ['A','B']  got ['B']  -> DATA LOSS

Under an unsynchronised race the loss is partial and varies, which is how I hit
it originally. Four processes each doing 50 distinct MERGEs against one
database, expecting 200 nodes:

expected 200  got 100  -> LOST 100 WRITES
expected 200  got 100  -> LOST 100 WRITES
expected 200  got 129  -> LOST 71 WRITES
expected 200  got 100  -> LOST 100 WRITES
expected 200  got 100  -> LOST 100 WRITES

Five runs, five failures, every child process exiting 0. The 129 run shows the
result can also be an interleaving of two snapshots rather than a clean
last-writer-wins.

With two writers instead of four it still occurs, just less often — 1 of 3 runs
in my testing — which is the awkward part: light concurrency looks fine in
testing and loses data later.

The unsynchronised race script is in the collapsed block below.

repro_race.py — four writers, no synchronisation
"""Minimal repro: two processes writing to one on-disk Grafeo database.

    python repro.py            # runs the whole thing

Expected: 100 nodes (2 writers x 50 distinct MERGEs).
Observed:  fewer, with no error raised by either writer.
"""
import shutil, subprocess, sys, tempfile, pathlib
import grafeo

DB = pathlib.Path(tempfile.gettempdir()) / "grafeo_mp_repro.db"
N = 50

if len(sys.argv) > 1:                     # child: writer role
    tag = sys.argv[1]
    db = grafeo.GrafeoDB(str(DB))
    for i in range(N):
        db.execute(f"MERGE (n:Item {{id:'{tag}-{i}'}})")
    db.close()
    sys.exit(0)

shutil.rmtree(DB, ignore_errors=True)
DB.unlink(missing_ok=True)
kids = [subprocess.Popen([sys.executable, __file__, t]) for t in ("A", "B", "C", "D")]
codes = [k.wait() for k in kids]

db = grafeo.GrafeoDB(str(DB))
got = list(db.execute("MATCH (n:Item) RETURN count(n) AS n"))[0]["n"]
print(f"grafeo {grafeo.__version__ if hasattr(grafeo,'__version__') else '?'}"
      f"  child exit codes {codes}  expected {4*N}  got {got}"
      f"  -> {'OK' if got == 4*N else 'LOST %d WRITES' % (4*N - got)}")

Version

0.5.42, Python (grafeo 0.5.42 from PyPI, cp312-abi3 wheel)
Python 3.14.7, Linux x86_64, glibc 2.44

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions