fix: eliminate silent event loss from queue corruption on shared storage (+ crash-safe recovery) - #78
Closed
Diego Colombo (colombod) wants to merge 12 commits into
Closed
fix: eliminate silent event loss from queue corruption on shared storage (+ crash-safe recovery)#78Diego Colombo (colombod) wants to merge 12 commits into
Diego Colombo (colombod) wants to merge 12 commits into
Conversation
This was referenced Aug 23, 2026
Diego Colombo (colombod)
added a commit
that referenced
this pull request
Aug 24, 2026
Final version of the stacked re-seat. #78 moved the server to 6.7.1; the storage-API layer holds it; this stacked head lands at 6.7.3. Adds the machine-readable migration manifest with the 6.7.3-maintenance-mode entry (forward-only structural rectification, no rollback machinery). SCHEMA_VERSION stays 1 -- the change is structural/operational, not a stored-shape change. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Diego Colombo (colombod)
added a commit
that referenced
this pull request
Aug 26, 2026
…ackend-neutral protocols Transplant PR #76's storage-API isolation onto PR #78's hardened durable queue so the two stack cleanly (78 -> 76). Every store is now reached only through a backend-neutral Protocol + factory; no consumer constructs a concrete backend or touches an on-disk path. queue_manager/ FileSystemQueueManager implements the QueueManager Protocol. Built on #78's authoritative body (the durable per-record cursor model): the class body is byte-identical to #78's queue_manager.py apart from the class rename and its self- references. protocol.py carries #78's Record/Batch verbatim; factory.create_queue_manager is the only queue backend selector. blob_store/ FileSystemBlobStore implements the BlobStore Protocol. write() returns a BlobReference (uri + size + last_modified); the store gains scan()/list() (async BlobReference iterators) and a fenced delete(uri, if_unmodified=ref). Adds settings.blob_backend. identity_store/ FileSystemIdentityStore implements the IdentityStore Protocol; the commit-order and fail-closed-load contract lives in the protocol. The backing path is private -- callers use exists(). Also folds in the blob-key fix: process_event includes tool_call_id in the blob-key node_id, so two same-millisecond parallel events no longer collide on one blob and silently overwrite each other. Consumers (registry, main, blob_processor, pipeline) build stores via the factories and pass URIs/references, never paths. The boot-reclaim dry-run log derives the blob path from the QueueManager's own queues_dir (fixing a latent mismatch when it differs from settings.queues_path); the identity first-boot warnings no longer echo the store path. The sole remaining config-path read outside the storage layer is registry.queues_dir_path -- the resolver the WriterLease boot detector uses precisely because it must not construct a QueueManager. Full non-neo4j suite: 2082 passed. Neo4j subsets (queue durability, blob ingest, identity auth): green. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Diego Colombo (colombod)
added a commit
that referenced
this pull request
Aug 26, 2026
Final version of the stacked re-seat. #78 moved the server to 6.7.1; the storage-API layer holds it; this stacked head lands at 6.7.3. Adds the machine-readable migration manifest with the 6.7.3-maintenance-mode entry (forward-only structural rectification, no rollback machinery). SCHEMA_VERSION stays 1 -- the change is structural/operational, not a stored-shape change. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
This was referenced Aug 26, 2026
…ain, safe boot, self-shrinking storage, operator GC) Hardens the event-ingestion server so it durably persists events under real shared-network-storage deployment conditions, recovers safely on restart, keeps its own on-disk storage bounded, and gives operators a safe reclaim tool. Public API contract is unchanged except for additive endpoints and /status fields. - Durable, corruption-free append: queue writes are atomic and single-writer-serialised per session under a per-key file lock, so concurrent/interrupted writes can no longer produce torn or merged log lines; a partial write is discarded and surfaced, never left to corrupt the tail. - Supervised draining: a dead drain worker is logged and its session recovered instead of silently stranded; poison/unparseable lines are dead-lettered and draining continues. - Safe fast boot: /status and /version answer from the first boot phase; pre-existing on-disk data is classified (resume-vs-remove), never crash-loops, and already-drained data is reclaimed at a bounded rate; boot progress is on /status.boot. - Self-shrinking queue storage: committed prefixes are reclaimed continuously as events drain (not only at session end); dead-letter files have bounded retention. - Writer-lease detector: detects two revisions briefly writing the same data directory during a rolling deploy and surfaces it on /status.writer_lease within a heartbeat (detector mode by default; never refuses boot). - Exactly-once idempotency fix: an idempotency key is recorded only after a durable write, so a failed write + client retry is honoured instead of being falsely refused as a duplicate. - Consistent graph-write concurrency: terminal graph flushes now use the same concurrency gate as all other writes. - Operator observability: previously-silent worker death/reap/cancel/lease-conflict/orphan transitions now emit structured, session-tagged log lines. - Operator storage GC: GET /queues/gc previews safe-to-delete fully-drained queue logs and expired dead-letters (deletes nothing, read scope); POST /queues/gc/apply performs a bounded, per-item re-verified deletion (write scope, refused until boot completes). Runs server-side — no disk or storage-key access needed. - Config: adds compaction/dead-letter-retention/GC/writer-lease settings; bounds the crash-recovery respawn cap and shortens the sweep interval. Deployment disk raised to 1 TB. Full test suite green (2070 non-Neo4j plus isolated-Neo4j), including adverse-state and crash-window cases. Docs updated (README API + settings, operational hardening runbook, Azure deployment, architecture overview + diagram). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
The GC preview/apply HTTP endpoints were not needed to address the durability work and exposed a destructive operation on the data-plane router without a proper security design. Removed the two routes, the scan/candidate enumeration, their config/model, and their tests. The automatic in-loop reclaim (continuous compaction + dead-letter retention) is unchanged and needs no endpoint. On-demand reclamation of a drained backlog, if ever needed, is an out-of-band maintenance operation run inside the container, not an exposed API. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Deletes an Azure Files deployed-mount smoke harness (for a test that is deliberately not run) and a throughput benchmark; neither is a correctness test for this change. Rewrites the comments and docstrings added by this branch into proper documentation -- describing what each test and code path does and why -- instead of tracing the internal design discussion. Comment/docstring and test-file-set changes only; no product logic changed. Full non-Neo4j suite green (2046 passed); ruff format/lint clean. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Cuts verbose explanatory comments down to what a reviewer needs: removes internal codenames, source-location references, change-history narration, and future-work notes; rewrites the two Neo4j reclaim/flush test files with concise docstrings. Comment/docstring text only -- no logic, names, or behaviour changed. Full non-Neo4j suite green (2046 passed); ruff format/lint clean. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Removes ~3600 lines of narrative/incident-history/codename/spec-tracking prose from comments and docstrings across the server and its tests, leaving terse purpose docstrings and only the load-bearing invariant/why notes a reader cannot infer from the code. Comment and docstring text only -- no logic, symbol names, string literals, assertions, or behaviour changed. Full non-Neo4j suite green (2046 passed); ruff format/lint clean; type:ignore/noqa pragmas preserved. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Bumps context-intelligence-server from 6.7.0 to 6.7.2 to reflect the durable ingestion hardening work on this branch: - Crash-safe queue - Supervised drain - Safe boot This new build is already installed and running as the live systemd daemon. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…rrency - offset reader accepts both the legacy JSON and bare-int forms; commit() unchanged; an unusable offset never silently re-drains from 0 - per-key idempotency lock: concurrent same-key POSTs append exactly once, store-after-append ordering preserved - exhausted poison batch containing session:end now finalizes (CompletedSession + delete_drained); no leaked drained log - compaction reclaims the committed prefix regardless of tail size; remove the tail-size cap - boot auto-reclaims provably-drained logs; an unreadable offset never deletes an intact log - partial-write rollback never truncates the queue - writer lease defaults to enforce, releases on clean shutdown, takes over a stale lease - add concurrent-append stress coverage (multi/single-session, >1MiB records) 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
- writer lease documented as enforce-by-default single-writer guard (refuses a live foreign writer, releases on clean shutdown, takes over a stale lease); drop the detect-default / "do not enable enforce" guidance - remove references to the removed /queues/gc operator endpoints - fully-drained logs are reclaimed automatically at boot; unresumable/reset-offset actions remain gated on reclaim_enabled - correct offset handling: an unparseable offset re-drains from byte 0 at any size, never deleted - drop the removed compaction tail-size cap; regenerate 05-durable-ingest-queue.png 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…recover finalize-orphans; keep dead-letters by default - boot serves /status immediately even when Neo4j is unreachable at startup: schema init moves into the guarded background boot as a retried phase, drainer start is gated on schema-ready, and an un-migrated graph is a visible failed boot rather than an ASGI-startup abort - per-phase boot timeout (boot_phase_timeout_seconds, default 300s) so a hung mount fails visibly instead of leaving /status.spool/metrics null forever - a finalize-orphan (tail-flush failure) now closes and deregisters the worker so a fresh event re-drains from the retained log, instead of a registered zombie only an opt-in sweep could recover - dead-letter auto-expiry ships off by default so an un-recovered event's last copy is never silently deleted 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…on /status A prior change deregistered and closed the worker on the two finalize-orphan paths (tail-flush failure). That broke the deliberate orphan-visibility design: a finalize-path orphan must stay registered so orphaned_sessions() surfaces it on /status (orphaned: true) and boot recover() re-enters it after restart. Restore the original behavior on both paths (no _safe_close / _deregister on orphan) and revert the unit tests that had been flipped to assert deregistration. Regression was only caught by the memory-capped Neo4j integration tests (tests/neo4j), which now pass. Also give the deterministic ~30s OOM recipe test explicit timeout headroom so it does not trip the global 30s per-test timeout during teardown. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
The drain path built a new AsyncGraphDatabase driver for every session_id with no pool bound, so bolt connections accumulated without limit until the server's bolt thread pool starved and ingest backpressured to clients. SessionRegistry now builds one shared, pool-bounded driver (lazily, on first session) and hands it to every Neo4jGraphStore it constructs. Neo4jGraphStore accepts an optional pre-built driver and tracks whether it owns it; close() only closes a driver it owns, so a per-session finalize can never take down the driver other live sessions are still using. The shared driver itself is closed exactly once, at lifespan shutdown. The pool-bounding kwargs (max_connection_pool_size, max_connection_lifetime) live in one helper in neo4j_store.py so the lifespan admin driver, the doctor CLI, and the registry's shared driver can never diverge -- this also avoids a registry->main import cycle, since main already imports registry. Config gains neo4j_max_connection_pool_size (default 50, well under the server's bolt thread-pool size) and neo4j_max_connection_lifetime (default 3600s, so idle connections recycle). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Route the process-wide cypher-query driver through the same bounded-driver helper as the admin and per-session drivers, so every process-wide pool shares one cap instead of leaving the query driver on an unbounded default pool. Driver construction now funnels entirely through the store module, so lifespan tests target that single construction site. Add evidence that the per-session driver leak is gone: structural tests prove N sessions build exactly one shared driver, that it is built with the bounded pool kwargs, that concurrent first-sessions cannot race into a second build, and that the shared driver is reclaimed exactly once and is idempotent to re-close. A live-Neo4j test drives many sessions through the real registry write path and asserts, via the server's own connection list, that open bolt connections stay bounded by the pool and are released on driver close. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Diego Colombo (colombod)
force-pushed
the
harden-durable-ingestion
branch
from
August 26, 2026 17:06
f4ef6b5 to
c4ad683
Compare
Diego Colombo (colombod)
added a commit
that referenced
this pull request
Aug 27, 2026
Extracts the queue-hardening slice from the squashed harden-durable-ingestion branch (PR #78), on top of the shared-driver base (PR #91). Writer-lease enforcement is a separate, later slice and is excluded entirely. - queue_manager.py / idempotency.py: taken wholesale from the source branch (near-rewrites: per-session file-lock record framing, dual-format committed-offset reader, continuous committed-prefix compaction, boot reclamation of provably-drained logs, per-key async idempotency lock). - registry.py: supervised drain loop with poison-batch isolation that finalizes correctly when a batch carries the terminal record, finalize retry on a non-drained delete, and the shared Neo4j driver wiring from PR #91 preserved unchanged. - main.py: boot backgrounded into schema -> heal -> reclaim -> expire -> reconcile -> seed -> topup -> sweep, each phase timeout-bounded, so a Neo4j-unreachable schema init no longer blocks first request; ingest's idempotency check/append/store sequence is serialized per key by an async lock (no lease involved); crash-recovery topup falls back to a session's byte-0 line when its head is unparseable. - config.py: queue self-bounding settings only (compaction thresholds, dead-letter retention, boot-phase timeout); no writer-lease settings. - status.py: BootState surfaced on /status (reclaim/resume/defer counters and boot phase); no writer-lease field. - handlers/data_layer_2/session.py: session:end no longer self-flushes -- the drainer's gated flush barrier is now the sole trigger; the ended-node upsert is seeded with the labels already read so it can't shadow a persisted terminal type. - tests: ported the concurrent-append stress suite (multi-session and single-session contention, >1 MiB records) and the queue adverse-state suites (boot safety/reclaim, drain supervision, finalize-delete ordering, idempotency-store-on-success, large-event tail drop, steady-state reclaim, drain lifecycle logging), all with writer-lease-specific cases removed. No writer_lease references remain anywhere in shipped code or tests. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Diego Colombo (colombod)
marked this pull request as draft
August 27, 2026 15:29
Contributor
|
Superseded by #92, which lands the same append-serialization fix in a smaller, reviewable form. Closing so there is a single authoritative PR for this work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Durable, corruption-free event-ingestion queue
What this is
The event-ingestion server appends session events to per-session on-disk logs on a mounted file volume (Azure Files in the cloud, a local mount in development), then drains them into Neo4j. This work makes that durable queue corruption-free and self-bounding under concurrent writes, and makes the single-writer guarantee enforced rather than merely observed.
The guarantee rests on one property: a single writer holds a per-session file lock across the whole record, so a record lands as exactly one newline-terminated byte range or not at all — independent of the underlying filesystem's concurrent-append semantics. Storage is reclaimed only by draining and then deleting drained data; the queue is never capped or truncated.
What it delivers
/status.Components added / modified
context_intelligence_server/queue_manager.py— per-session write serialization and record framing; dual-format committed-offset reader; continuous committed-prefix compaction (no tail-size cap); boot classification and reclamation; non-truncating partial-write rollback.context_intelligence_server/registry.py— supervised drain loop; poison-batch isolation that finalizes when the batch carries the terminal record.context_intelligence_server/idempotency.py— idempotency cache plus a per-key async lock registry.context_intelligence_server/main.py— ingest endpoint with lease-serialized idempotency; boot reconciliation that automatically reclaims provably-drained logs; lease release on clean shutdown.context_intelligence_server/writer_lease.py— single-writer lease with enforce / detect / off modes, heartbeat, stale-takeover, and owner-gated release.context_intelligence_server/config.py— settings for compaction, dead-letter retention, and the writer lease (enforce by default).tests/— a new concurrent-append stress suite (multi-session and single-session contention, records over 1 MiB), plus adverse-state coverage for offset back-compat, idempotency serialization, terminal-in-poison-batch finalization, uncapped prefix compaction, boot reclamation, non-destructive rollback, and lease enforce / stale-takeover / release.Validation