Skip to content

fix: share SQLite storage across LCM engine clones - #500

Closed
grantjayy wants to merge 3 commits into
stephenschoettler:mainfrom
grantjayy:fix/shared-sqlite-clone-storage
Closed

grantjayy wants to merge 3 commits into
stephenschoettler:mainfrom
grantjayy:fix/shared-sqlite-clone-storage

Conversation

@grantjayy

Copy link
Copy Markdown

Summary

LCM engine clones currently construct a fresh set of SQLite-backed helpers. A clone therefore pays database initialization cost again and retains another set of SQLite file descriptors even though it is serving the same logical database as its prototype.

This change gives each clone family one reference-counted storage bundle while keeping mutable engine/session/model state clone-local. Independently constructed engines still own separate bundles, but bundles resolving to the same canonical database path share one in-process reentrant lock for helper construction, operations, backup/commit, and final teardown.

Fixes #463.

Why this is needed

Hermes creates engine clones for child agents. Before this change, ten retained clones added 60 file descriptors on the measured macOS checkout and spent roughly 100 ms constructing duplicate SQLite helper sets. That multiplication raises the risk of descriptor exhaustion and repeated schema/WAL setup under parallel child-agent workloads.

The ownership boundary is the important part of the fix:

  • clones share only the storage helpers;
  • session IDs, cursors, runtime counters, model/provider metadata, adaptive retrieval state, and assertion extractors remain engine-local;
  • every engine owns exactly one idempotent lease;
  • owner-first shutdown releases one lease without closing storage still used by clones;
  • final release attempts every helper close and reports combined failures;
  • abandoned clones release their lease through finalization;
  • failed clone construction rolls its acquired lease back.

This is complementary to #470's cleanup work. It does not replace #486's cross-process FTS bootstrap locking; the lock introduced here is process-local.

Implementation

  • Add a reference-counted _SharedStorage bundle for message store, summary DAG, lifecycle state, optional assertion store, and optional query-view store.
  • Pass the bundle into clone_for_agent() and preserve clone-local runtime/model state.
  • Use a weak canonical-path lock registry so independently constructed bundles for the same database serialize helper initialization, operations, backup/commit, and teardown without sharing mutable bundle ownership.
  • Allow each SQLite helper to adopt the shared reentrant lock.
  • Synchronize every touched store/DAG operation, lifecycle debt cleanup, backup, close, and commit path that uses a shared connection.
  • Add lifecycle, concurrency, rollback, finalization, profile-rebinding, aggregate-cleanup, and descriptor-regression tests.
  • Add scripts/measure_clone_storage.py for reproducible before/after measurements against a selected checkout.

Reproduced benchmark

Environment: macOS, Python from the active Hermes environment, 25 samples, 10 retained clones. The benchmark imports each selected checkout using LCM_BENCH_REPO_ROOT.

Commands:

LCM_BENCH_REPO_ROOT=/private/tmp/hermes-lcm-baseline-check-20260804 \
  python scripts/measure_clone_storage.py --samples 25 --clones 10 --json

LCM_BENCH_REPO_ROOT=/private/tmp/hermes-lcm-shared-storage-impl-20260804 \
  python scripts/measure_clone_storage.py --samples 25 --clones 10 --json

Commits:

  • Base: 6b7dbb1
  • Feature: 1708241c1ce9c759348509f9148b60e0f9fc02ff

Results:

  • Retained-clone file-descriptor delta: +60 → 0
  • Median clone setup: 5.877985 ms → 0.198782 ms
  • Ten-clone setup: 100.084453 ms → 2.203067 ms
  • Median initial startup: 6.731055 ms → 7.368695 ms

Raw local receipts:

  • /private/tmp/lcm-storage-benchmark-final-base-20260804.json
    • SHA-256 2bcd2a9430f2d63a5b4f7ef00a9386157a9475e27825d935c44e03d3d3c09c06
  • /private/tmp/lcm-storage-benchmark-final-feature-20260804.json
    • SHA-256 ba969980fa6087ded140a811b1bcf91a2b725e9f64aae536f1484f7cc84a7626

Verification

Green feature/affected gates:

  • tests/test_lcm_core.py::TestLCMEngineSharedStorage: 9 passed
  • tests/test_lcm_engine.py: 751 passed, 1 skipped
  • tests/test_lcm_core.py excluding one independently reproduced upstream failure: 316 passed
  • tests/test_packaging_install.py: 38 passed in the grouped run before the known isolated baseline failure was reproduced
  • tests/test_assertion_lifecycle_tools.py tests/test_assertion_store.py tests/test_query_view_store.py: 49 passed
  • tests/test_lcm_core.py::TestLifecycleStateStore: 19 passed
  • tests/test_assertion_store.py: 20 passed
  • tests/test_query_view_store.py: 21 passed
  • Ruff on every changed Python file: passed
  • Python compilation on changed production/benchmark files: passed
  • git diff --check: passed
  • Independent adversarial concurrency/lifecycle review: PASS after two repair rounds

Full-suite result:

  • The suite remains red on current main for pre-existing or load-sensitive tests. The observed failures were reproduced against pristine 6b7dbb1, including strict wall-clock embedding deadlines, macOS /var versus /private/var containment assertions, trajectory token/chunk expectations, provider-routing behavior, and an isolated packaging registration test.
  • Three doctor failures initially exposed an incompatible post-shutdown alias-clearing change. That change was reverted; all three doctor tests pass on the final feature commit.
  • No remaining full-suite failure was unique to this feature branch in the bounded base comparison.

Scope

This PR intentionally does not:

  • share one bundle across independently constructed engines;
  • provide cross-process locking;
  • change session/runtime/model ownership;
  • change public configuration;
  • include the separate reasoning-control or preflight-maintenance features.

@grantjayy

Copy link
Copy Markdown
Author

Superseded before review: the first head branch accidentally included two local planning-only commits. Reopening from a clean branch based directly on current main; production and test commit content is unchanged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: agent clones multiply SQLite file descriptors

1 participant