Repository navigation
fix(storage): never hold a read transaction across a StreamActivities send - #1743
Merged
Merged
Conversation
… send StreamActivities walked the activity bucket inside one BBolt read transaction and sent each record into a buffered channel. #1737 made the send cancel-aware, but a consumer that stops reading without cancelling (an activity export to a client that keeps the connection open but no longer reads) still parked the producer inside db.View. The open read transaction pinned the mmap: the next write that had to grow the file waited on the remap lock for ever, and every later reader — agent-token lookups included — queued behind that writer, hanging all authenticated requests while /healthz kept answering. The producer now collects a bounded batch (at most 100 matches, at most 1000 keys examined) inside a short db.View, leaves the transaction, then sends, and resumes the walk strictly before the last examined key in a new transaction. Offset, limit and newest-first order are unchanged; records written after the stream starts are not guaranteed to appear, and records pruned between batches are skipped. Tests: the no-cancel test that asserted the leak as the contract is replaced by one that asserts a stalled consumer holds no transaction, a remap test (a 16 MiB write must not block behind a stalled stream), and batch-boundary coverage for offset/limit/order and a sparse filter past the per-transaction scan cap.
Dumbris
enabled auto-merge (squash)
October 23, 2026 05:17
Deploying mcpproxy-docs with
|
| Latest commit: |
aa34be8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe3fc6cc.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-issue-1742-fix-91b393.mcpproxy-docs.pages.dev |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 39904914281 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
closes #1742
Hazard
storage.Manager.StreamActivitieswalked the activity bucket inside one BBolt read transaction and sent each record into a buffered channel. #1737 made the send cancel-aware, but a consumer that stops reading without cancelling (an activity export to a client that keeps the connection open but no longer reads) still parked the producer insidedb.View. The open read transaction pinned the mmap: the next write that had to grow the file waited on the remap lock for ever, and every later reader (agent-token lookups included) queued behind that pending writer. Result: all authenticated requests hang while/healthzkeeps answering — the #1742 signature.Fix
The producer never holds a read transaction across a channel send. Each short
db.Viewcollects at most 100 matching records (examining at most 1000 keys), leaves the transaction, then sends; the next batch resumes strictly before the last examined key (Seek+Prev, robust to that key being pruned). Offset, limit and newest-first order are unchanged. Records written after the stream starts are not guaranteed to appear, and records pruned between batches are skipped.Tests
TestStreamActivities_StalledConsumerHoldsNoTransaction(replaces the test that asserted the leak as the contract): a consumer that stops reading without cancelling leavesOpenTxN == 0.TestStreamActivities_StalledConsumerDoesNotBlockRemap: with a stalled stream, a 16 MiB write (forces an mmap remap) completes, and the stream then delivers all 300 records.TestStreamActivities_BatchBoundaries: order, offset/limit across batches, and a sparse filter past the per-transaction scan cap.Both stalled-consumer tests fail on
main(the remap test times out at 5 s) and pass here.storage,runtime,httpapipass with-race; golangci-lint v2 clean (bare and-tags server).Live verification
Scratch core with a seeded DB (12k activity records, ~1 MB below the mmap boundary), a stalled export client, then a loop of writes + authenticated reads:
main: writes and authenticated reads hang after ~200–290 writes; the goroutine dump matches the issue (StreamActivities.func1.1parked inView, a writer in(*DB).mmaponRWMutex.Lock, readers inbeginTxRLock).Cross-reviewed by opencode
gpt-6.1-sol: CLEAN.