Repository navigation
server: cap reports, exports and heavy admin reads below the participation path - #3045
Merged
Merged
Conversation
…ed pool Reports, the data-export CSV routes, the comment-clusters export, conversationStats and the moderator participant query now use a small, timeout-bounded connection pool, kept separate from the pools the participant-facing path uses (vote and comment writes and next-statement selection on the read-write pool; the math blob read, GET /votes and participation reads on the read pool). A flood of slow reports or exports can no longer take the connections votes, comments and the next statement need: an export that cannot get one of the pool's connections within the acquire timeout is rejected rather than waiting on, or spilling into, a participant-facing connection, and each statement on the pool is capped by a statement timeout. The pool size, acquire timeout and statement timeout are configurable (REPORT_EXPORT_POOL_MAX, REPORT_EXPORT_ACQUIRE_TIMEOUT_MS, REPORT_EXPORT_STATEMENT_TIMEOUT_MS) and documented in docs/configuration.md, alongside the per-workload connection budget. Also hardens the shared pooled-query helper: a failed or timed-out connection acquire previously threw a TypeError that masked the real error, because it called a release callback the pool does not provide on that path. A load guard (participation-pg/export-budget.pgtest.ts) saturates the export pool and proves a vote, a comment and the next-statement call stay prompt while a further export is rejected. It runs in the existing Postgres result-reads CI job. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NVGBuYCk4EZmiz4csUv9s
…xport pool, free export connections on client disconnect - Update the export and empty-conversation unit mocks to the dedicated report/export query methods, so the senders' existing assertions hold and prove the senders read on the report/export pool. - Run the owner-gated GET /participation whole-conversation group-by scans on the report/export pool, the same class as conversationStats; getXids stays on the read pool (shared helper, bounded paginated join). - Release the report/export connection when the HTTP client disconnects mid-stream: close the cursor and stop fetching so an aborted download cannot hold one of the few export connections. Success-path output is unchanged. Add a guard proving a disconnected export frees its connection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NVGBuYCk4EZmiz4csUv9s
…t/export methods The vote-sign export byte test mocks pg-query and exercises the report senders, which now read on the dedicated report/export pool. Point its mock at queryP_reportExport / stream_queryP_reportExport so its assertions hold, the same fix as the other two export unit suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NVGBuYCk4EZmiz4csUv9s
…efault REPORT_EXPORT_POOL_MAX now defaults to 1, so further exports wait for the acquire timeout and the primary keeps more room for participation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NVGBuYCk4EZmiz4csUv9s
Delphi Coverage Report
|
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
1 participant
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.
Observable changes (please rule on these before merge)
No report or export output changes. What changes is how reports and exports acquire database connections, and so their behaviour under load:
/api/v3/dataExportCSV routes, the comment-clusters export,conversationStats, the moderator participant query and the owner-gatedGET /api/v3/participationstats route now run on a small dedicated connection pool (default 1 connection per server process, so they run one at a time) instead of the shared read pool. The moderator participant query additionally moves off the read-write (vote) pool it used before, where it ran a heavy CTE alongside vote and comment writes — a direct win for the write path. Under heavy concurrent report/export load, a report or export that cannot get one of those connections within the acquire timeout (default 30s) is now rejected with an error, where before it would wait on the shared read pool. That is the point: it keeps that load off the connections votes, comments and the next statement need.REPORT_EXPORT_STATEMENT_TIMEOUT_MS).conversationStats, the moderator participant query and the owner-gated participation-stats route (all admin/owner-only) now share the small export pool, so under concurrent exports they may queue or, past the acquire timeout, fail — admin/owner reads, never the participant-facing path.I could not measure production export sizes or durations, so the defaults are generous; raise them per deployment if needed. The pool points at the same database as the read pool — no new host, no replica.
Why
The participant-facing path — submitting a vote, submitting a comment, and getting the next statement — must always answer and must never be made to wait or be rejected because of other work. Today the data-export routes stream whole conversations on the shared read pool, which the next-statement call also uses (the math blob read and
GET /votes). This moves the heavy, whole-conversation reports, exports and admin/owner scans onto their own small, bounded pool, so those can no longer take the read-pool connections the participation path needs.One report-family read stays on the read pool by design: the xid report export (xid report route →
getXids). It is a paginated, indexed join bounded byLIMIT/OFFSET, not a whole-conversation scan, andgetXidsis a shared helper also called by the owner participation-stats route, so it was left on the read pool. The accurate guarantee is therefore: the whole-conversation streaming exports and admin/owner scans can never take the read-pool connections; the xid export remains a small, bounded read on the read pool (owner-gated, far below the risk of the public streaming exports this isolates).What changed
REPORT_EXPORT_POOL_MAX(default 1), with an acquire timeout and a per-connectionstatement_timeout; newqueryP_reportExport/stream_queryP_reportExport.report.ts), the comment-clusters export,conversationStats, the moderator participant query and the owner-gatedGET /api/v3/participationstats route (its two whole-conversation group-by scans) route through them. The moderator participant query additionally moves off the read-write (vote) pool. Participant-facing reads are unchanged.TypeError(it called a release callback the pool does not provide on that path), masking the real error; it now rejects cleanly.docs/configuration.md, with the per-workload connection budget.How the participation path stays clear
Votes and comments write on the read-write pool; the next-statement call reads on the read-write pool (candidate selection) and the read pool (math blob,
GET /votes). None use the new export pool. The guardserver/__tests__/participation-pg/export-budget.pgtest.tssaturates the export pool with slow queries and asserts a vote, a comment and the next-statement call each answer well under budget (~20 ms observed) while a further export is rejected at the acquire timeout. A second guard in the same file shows a streamed export that is disconnected mid-stream frees its connection so a further export is served immediately. Both run in the existing "Postgres Delphi result reads" CI job and are not skippable.Tests
The report/export unit suites (
exportRoutes,emptyConversationConsumers) are updated to the new query methods and pass. The Postgres reads suite (Postgres 17 + migrations) runs 7 suites / 44 tests, including the result-pool concurrency guard and the export-pool guard. With the pool capped at 2 for the test, the export-pool guard reports:{export_pool_max:2, export_connections_held:2, acquire_timeout_ms:1500, vote_ms:26, comment_ms:20, nextcomment_ms:21, overflow_export_ms:1526, overflow_export_rejected:true}{disconnect_test:true, export_pool_max:2, holders:1, stream_ended_normally:false, probe_ms:6, probe_served:true}— a further export is served in 6 ms after a mid-stream disconnect frees the connection.🤖 Generated with Claude Code
https://claude.ai/code/session_018NVGBuYCk4EZmiz4csUv9s