Skip to content

fix(storage): give background tasks their own connection + process-level write lock + explicit transactions #36

Description

@ch-liuzhide

Context

Goal axis: reliability (可靠). Under real multi-agent load the storage layer is the concurrency/atomicity ceiling. Every FastAPI handler and the background SchedulerManager (consolidation + forgetting) share one aiosqlite connection, and the KnowledgeGraph is a single in-process nx.Graph. This issue tracks the SQLite-correctness work that closes audit defect C1.

PR #24 already landed a large share of the original C1 remediation. This issue is deliberately narrowed to what is still open after that PR. The longer-term scale story (Postgres/pgvector, pluggable vector store) is a separate discussion — roadmap D4 — not part of this issue.

Already landed in PR #24 (verified, not in scope here)

  • Process-wide write lock + explicit transactions in the SQLite store: SQLiteMemoryStore._write_lock (src/hebb/storage/sqlite_store.py:68); _begin() issues BEGIN IMMEDIATE (src/hebb/storage/sqlite_store.py:75-83); every multi-statement write (create, update, delete, delete_expired, update_access, update_access_batch, update_embedding, update_expiry, update_expiry_batch) wraps the body in try: … commit() except BaseException: rollback() (e.g. src/hebb/storage/sqlite_store.py:117-153, 247-262, 544-578). The audit's orphan-row scenario (write F2) and "commit another coroutine's half-finished transaction" (write F1) are addressed for the store's own writes.
  • Unified KG lock: KnowledgeGraph.lock (src/hebb/graph/knowledge_graph.py:37). The previously-unsynchronised session consolidation graph write (audit 遗忘F4) is now under the lock — src/hebb/agents/consolidation_agent.py:374-375 and 393-405; standalone _consolidate_one under the same lock at src/hebb/agents/consolidation_agent.py:637-646; the forgetting sweep purges + saves under self.knowledge_graph.lock at src/hebb/scheduler/manager.py:189-194.
  • Consolidation runs are serialised against the admin route via SchedulerManager._consolidation_lock (src/hebb/scheduler/manager.py:50, taken at 120).
  • Orphan reconciliation routine: KnowledgeGraph.reconcile() (src/hebb/graph/knowledge_graph.py:285-324).
  • A first concurrency test exists: test_concurrent_creates_do_not_interleave (tests/unit/test_audit_storage.py:90-106).

Current state — what remains (verified file:line)

  • Single shared connection is still the design. _create_sqlite() opens one aiosqlite connection and hands the same object to both the MemoryStore and the PartitionStore; the in-code comment is explicit: "Resilient pragmas on the single shared connection … Serialization itself is handled by the store's in-process write lock (INT-2), not by re-architecting to a pool."src/hebb/storage/factory.py:46-57. PR fix(core): 0.1.7 — audit remediation + eval harness + custom HTTP embedding #24 added busy_timeout=5000 + reaffirmed WAL (factory.py:52-53) but did not give background tasks their own connection. The same store/connection is shared by the FastAPI handlers and the SchedulerManager (constructed with the shared memory_store/partition_store/knowledge_graphsrc/hebb/scheduler/manager.py:32-50).
  • Serialization is split across three independent locks, not one. SQLiteMemoryStore._write_lock (src/hebb/storage/sqlite_store.py:68), KnowledgeGraph.lock (src/hebb/graph/knowledge_graph.py:37), and SchedulerManager._consolidation_lock (src/hebb/scheduler/manager.py:50) are distinct asyncio.Lock instances. A consolidation step that creates a memory then mutates the graph holds the write lock for the SQL insert and the KG lock for the graph write as two separate critical sections (src/hebb/agents/consolidation_agent.py:362-375) — the SQL row and its graph reference are not committed atomically, so a crash between them still produces a graph orphan (the very class reconcile() exists to clean up after).
  • No combined write/consolidate/forget stress test. tests/unit/test_audit_storage.py:90 only exercises concurrent creates; there is no test driving interactive writes, a consolidation batch, and a forgetting sweep concurrently and asserting zero orphan / zero lost rows.

Proposed approach

  1. Give background tasks their own connection. Have create_stores() provide a second connection (or a small dedicated handle) for SchedulerManager so consolidation/forgetting do not share the request connection. The eval harness already runs consolidation against an isolated workdir db (eval/cli.py, per-scenario isolated hebb.db) — reuse that "own connection" capability rather than inventing a new path. Alternatively, if a single connection is kept, route all writes (store + graph) through one process-level lock so the split-lock gap above is closed.
  2. Make multi-statement operations explicitly transactional with rollback. Mostly done in the store; audit the remaining batch paths in manager.py (forgetting batches) and the consolidation agent so each logical unit either commits as a whole or rolls back.
  3. Bring every KG read-modify-write + save() under one lock with consistent discipline, and ensure the SQL mutation and its corresponding graph mutation share a single critical section (or are otherwise crash-consistent) so an interrupted consolidation cannot leave a graph orphan.

Acceptance criteria

  • Background tasks (consolidation, forgetting) no longer share the FastAPI request connection — they use their own connection — or all writes (SQL + graph) are serialised behind one process-level lock.
  • Every multi-statement operation across the store, scheduler, and consolidation agent is wrapped in an explicit BEGIN IMMEDIATE … COMMIT with rollback on failure.
  • KG read-modify-write + save() is consistently guarded, and a SQL write plus its graph mutation form a single crash-consistent unit (no new orphan on an interrupted consolidation step).
  • A concurrent write/consolidate/forget stress test produces no orphan and no lost rows (extends tests/unit/test_audit_storage.py beyond the create-only case at line 90).

Scope / out of scope

  • In scope: SQLite-correctness — connection ownership for background tasks, explicit transactions, and consistent KG lock discipline.
  • Out of scope: The storage-backend scale strategy (SQLite single-connection ceiling vs Postgres/pgvector vs pluggable vector store) — that is roadmap D4 and decides the direction, not the correctness fix here.

References

  • reports/audit/core-system-audit-2026-06-07.md — defect C1 (单一共享 sqlite 连接 + 无锁共享 graph) and its 统一修复方向.
  • reports/audit/newuser-experience-audit-2026-06-08.md
  • reports/design/capability-gap-roadmap-2026-06-11.mdH4 (this issue) and D4 (storage-backend strategy).

Filed from the capability-gap roadmap (reports/design/capability-gap-roadmap-2026-06-11.md).

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: reliabilityConcurrency, data integrity, robustness / 可靠effort: hardSubstantial engineering: ~1-4 weeks

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions