Repository navigation
feat(store): move the transcript archive flush off the commit path (RIG-5158) - #2223
Merged
Merged
Conversation
…IG-5158) AppendTranscriptEntry ran the compaction flush and the safety valve inline, so CommitConversationFrame blocked on the object-store PUT. A slow or frozen store stalled the agent's serial frame chain, and each commit gave up at the agent's 5s attempt timeout. AppendTranscriptEntry now commits the row and asks the Store's flusher for a pass. The flusher runs one pass at a time per session, at most 4 at once, each under a 60s deadline. A failed pass is logged, counted in compass.store.transcript_flush_failures, and covered by the next commit. Open starts the flusher; Close joins every pass before closing the pool. Commits can now land during a pass, so the database guards the two new races. The valve finds the latest checkpoint inside its size query and never cuts past a checkpoint row. A pruning flush rolls back unless it deletes exactly the rows it archived. SafetyValveSegments hides valve segments at or below the latest checkpoint. FlushTranscript is the synchronous barrier tests use. The e2e s3 outage leg keeps gating on the committed row. Implements docs/designs/agent/compass-transcript-flush-off-commit.md (DL-491). Ledger-impact: none Co-authored-by: Matt Wilkinson <matt@rigel.build>
Ledger-impact: none Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
😎 This pull request was merged. |
Contributor
Author
|
Review loop at head c040cfc: round 1 found 0 high, 1 medium, 1 low. The medium was a missing global-cap test, now TestFlushPassesShareGlobalCap; the low was a stale SegmentKindSafetyValve comment. Both are fixed. Round 2 found 0/0/0. After review, the pre-push hook's nilaway flagged valveCut slicing a possibly-nil tail, so an empty-rows guard was added. TestValveCut passes and nilaway is clean. |
|
Compass engineering docs preview: https://compass-repo-rig-5158-flush-l8mp.compass-eng-docs.pages.dev Deployed from |
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.
Summary
Move the transcript archive flush off
CommitConversationFrame. The commit returns once the row lands in Postgres. A Store-owned flusher archives later, one pass at a time per session.Why
AppendTranscriptEntryran the compaction flush and the safety valve inline. The object-store PUT had no deadline. A slow or frozen store held the RPC until the agent's 5s attempt timeout, and the agent's serial frame chain stalled behind it. This is the measured root cause of the s3-outage e2e flake (RIG-5157).Change
Implements the frozen record
docs/designs/agent/compass-transcript-flush-off-commit.md(DL-491), both tasks.AppendTranscriptEntrycommits the row and callsrequestFlush.primaryFlushis gone. A deduped retry, or a store with no object store, requests nothing.go/internal/store/transcript_flusher.go):maxConcurrentFlushes = 4passes at once;compass.store.transcript_flush_failures.Openstarts the flusher.Closecancels it and waits for every pass beforepool.Close().HotTailBytesandHotTailSizesfind the checkpoint inside the statement, andvalveCutstops before any checkpoint row.PruneTranscriptEntriesis now:execrows. A pruning flush rolls back witherrFlushRacedunless it deleted exactly the rows it archived.SafetyValveSegmentshides valve segments at or below the latest checkpoint.FlushTranscriptis the synchronous barrier tests use. The sync-flush tests, the server resume S3 test and the e2e comments are updated.Verification
New pgtests pass, and pass 5 of 5 under
-race:Closejoins the pass;valveCutcases;CommitConversationFramereturns while the PUT hangs.Mutations, each killed by its test:
valveCutcheckpoint stop removedTestValveCutCloseskips the joingo test -tags pgtest -racepasses for the fullinternal/store,internal/runnerhubandserverpackages on a fresh Postgres.golangci-lintreports 0 issues with and without the full tag set.sqlc-driftis clean.go build -tags podman ./e2e/...succeeds.Review loop: round 1 found 0 high, 1 medium, 1 low; both are fixed. Round 2 found 0/0/0.
Refs RIG-5158, RIG-5157.
Ledger-impact: none (implements DL-491, already recorded).