feat: concurrent-writer driver with convergence verification (bm#1248) - #42
feat: concurrent-writer driver with convergence verification (bm#1248)#42phernandez wants to merge 1 commit into
Conversation
Adds 'bm-bench run concurrent-write': N independent bm mcp client sessions (the basic-memory#1214 field-report shape) create and edit notes in one shared project with overlapping relation targets and contended hub notes, capturing per-op latency/errors and throughput, then verifying convergence against the on-disk files and the run's isolated SQLite index: file/entity counts, duplicate permalinks, duplicate observation/relation tuples, and per-line bmk-* markers that detect both lost and doubled writes. Report-only by default (divergence is a valid benchmark result for v0.22.1); --strict exits nonzero for gating. just targets: bench-write-smoke, bench-write-load. Refactor: _WarmMcpClient, _resolve_bm_command_prefix, and _status_json_is_ready move from providers/bm_local.py to a shared bm_runtime.py (provider delegates; behavior unchanged) so the driver and the retrieval provider share one bm CLI/MCP contract. Fairness/reproducibility: same external bm contract for every version under test, fresh isolated home per run (BASIC_MEMORY_CONFIG_DIR plus sandboxed BASIC_MEMORY_HOME), BASIC_MEMORY_* env passthrough for axis toggles, full provenance in manifest.json. Postgres row-integrity queries are a follow-up; the workload itself is DB-agnostic. Validated end-to-end at 4 writers x 25 notes against basic-memory main (ca5c54a8): 193/194 ops ok, zero duplicate rows, zero deadlocks; the marker check surfaced 3/45 concurrent hub appends acknowledged by edit_note but absent from file and DB (last-writer-wins overwrite across MCP sessions) - the class of signal the driver exists to catch. Refs basicmachines-co/basic-memory#1248 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4rbaeHJN3L7CREp5v38J9 Signed-off-by: phernandez <paul@basicmachines.co>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b435ed0036
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if config.measure_reindex: | ||
| console.print("Measuring full reindex wall time...") | ||
| reindex_start = time.monotonic() | ||
| run_command(prefix + ["reindex", "--search", "-p", config.project_name], env=env) | ||
| reindex_seconds = time.monotonic() - reindex_start |
There was a problem hiding this comment.
With the default --measure-reindex, this command reindexes the project before querying the SQLite database for convergence. Consequently, the reported verdict describes the post-reindex state rather than the state reached when the concurrent workload settled; any rows repaired, removed, or introduced by reindexing can mask the concurrency defect or create a false one. Capture integrity immediately after _settle_index, then run the optional reindex timing separately.
Useful? React with 👍 / 👎.
| if self._thread.is_alive(): | ||
| self._requests.put(None) | ||
| self._thread.join(timeout=self._startup_timeout_seconds) | ||
| self._thread = None |
There was a problem hiding this comment.
When an operation exceeds request_timeout_seconds, its MCP call remains in flight, but stop() waits only the 30-second startup timeout and then discards the thread reference even if the thread is still alive. A sufficiently hung write can therefore complete while _settle_index, reindex, or integrity verification is running, making the resulting artifacts race with an untracked writer; stop must cancel/terminate the subprocess or confirm the thread has exited before returning.
AGENTS.md reference: AGENTS.md:L111-L117
Useful? React with 👍 / 👎.
| created_at_utc=utc_now_iso(), | ||
| benchmark_git_sha=git_sha(Path(".")) or "unknown", | ||
| bm_source=config.bm_source, | ||
| bm_resolved_sha=git_sha(Path(config.bm_local_path)) if config.bm_local_path else None, |
There was a problem hiding this comment.
Whenever --bm-local-path is omitted—the documented default and both new just targets—this unconditionally records bm_resolved_sha as null even though the actual executable may be any installed basic-memory>=0.20.2; the manifest simultaneously labels its source as GitHub main. Published results therefore cannot identify or reproduce the BM implementation being compared, so resolve the installed executable's revision or require a pinned source.
AGENTS.md reference: AGENTS.md:L78-L83
Useful? React with 👍 / 👎.
|
Superseded by basicmachines-co/basic-memory#1304. The benchmark suite is canonical in Core under This repository is being archived. |
1 participant
Adds the concurrent-writer driver required by basicmachines-co/basic-memory#1248 (axes 1 and 4): a
bm-bench run concurrent-writecommand that runs N independentbm mcpclient sessions writing and editing notes in ONE shared Basic Memory project — the basic-memory#1214 field-report shape — and then verifies the project converged.What it does
--notes-per-writernotes (observations + relations into sharedtopic-*/hub-*pools for overlapping-entity contention) and, per--edit-ratio, appends to shared hub notes (max contention) and to its own earlier notes. Deterministic per--seed;--max-secondsoptionally caps the phase.per-op.jsonl), per-op-type mean/p50/p95/max, throughput (notes/min, ops/s), error taxonomy (deadlock / sqlite_locked / timeout / write_conflict / other), terminal writer failures, settle time, full-reindex wall time.(entity, category, content)observation tuples (the #1214 duplication metric, plus a redundancy % figure)(from, to, type)relation tuplesbmk-*marker, so lost appends and doubled appends are both detectedbenchmarks/runs/<run-id>/:manifest.json(provenance incl. BM SHA),per-op.jsonl,concurrent-write-summary.json,summary.md.--strictmakes it exit nonzero when checks fail (used byjust bench-write-smoke).Refactor
_WarmMcpClient,_resolve_bm_command_prefix, and_status_json_is_readymoved fromproviders/bm_local.pyinto a sharedbm_runtime.py(provider delegates; behavior unchanged) so the driver reuses the same warm-MCP/stdio contract instead of duplicating it.Fairness / reproducibility
bmCLI + MCP contract for every BM version under test (--bm-local-pathfor checkouts);write_note/edit_notesignatures are identical between v0.22.1 and main.benchmarks/.bm-homes/(BASIC_MEMORY_CONFIG_DIR+ sandboxedBASIC_MEMORY_HOMEso the default project doesn't index operator notes).BASIC_MEMORY_*env passes through, so Redis/semantic axes are toggled the same way as the retrieval scripts.Validation (small scale, current main)
Run
cw-validate-main-r1— 4 writers x 25 notes (194 ops: 100 creates + 90 appends + 4 hub setups) against basic-memory main @ca5c54a8, SQLite:sqlite3.OperationalError: database is locked(recorded assqlite_locked, correctly excluded from expected state)no_lost_writesfound 3 of 45 concurrent hub appends thatedit_noteacknowledged as success but that are absent from the final file AND from every DB table (verified manually). All three are same-hub appends racing other writers, i.e. an acknowledged append lost to a last-writer-wins overwrite across separatebm mcpprocesses. Whether that is accepted LWW semantics or anedit_noterace worth fixing is a #1248/product question — exactly the kind of thing this driver exists to surface.The strict gate (
just bench-write-smoke) exits nonzero on divergence; report-only is the default since divergence is the expected v0.22.1 result.Unit tests cover plan determinism, marker uniqueness, edit-target validity, latency stats, error classification, and the integrity checks against synthetic SQLite fixtures (clean, duplicated-observation, lost-write, duplicate-permalink cases).
Refs basicmachines-co/basic-memory#1248
🤖 Generated with Claude Code
https://claude.ai/code/session_01G4rbaeHJN3L7CREp5v38J9