[FumingPower3925]

Flakes lane, package D (+ F2): driver/postgres TestStreamingLarge measured the wrong path and the wrong moment; and the Query_1col_1row allocation budget is a per-path number. Two test commits, no driver change. Refs #1104

D. TestStreamingLarge (stream_test.go)

Failure: "heap grew 11.3 MB, exceeds 10 MB budget — likely not streaming" (CI, a dependabot PR, green on rerun). I reproduced it: the package with -race -cover -count=8 fails 6 of 8 iterations, always with 11,9xx,xxx bytes (11.4 MB).

Cause, from a heap profile diff (before/after the query) of a failing iteration: the whole 11.35 MB is pgConn.QueryContext -> simpleQuery -> pollSync -> pgSyncLoop.WriteAndPoll -> onRecv -> appendBuffered -> appendRowFromAlias. That is the SYNC fast path: when the poll on the caller's goroutine takes the whole reply, syncMode is on and every row is buffered, and the request never streams. Whether the poll takes all 100K rows in one go is timing, so on a slow instrumented run it does (the buffered result, 100K x about 119 bytes, is then held by the request pool, and the test's single runtime.GC() leaves it in sync.Pool's victim cache) and on others the poll gives up early and the request streams. At 100K rows (under the 64 MiB maxDirectResultBytes) that buffering is within what the driver promises; the test assumed one path and measured retained heap after the rows were closed.

Fix: the test runs the conn without its sync fast path (conn.syncLoop = nil, which is what a platform without one has), so the request streams on every run, which is the property under test; it samples the LIVE heap (two GCs, so pool victims go too) every 5000 rows while the rows are consumed and once at the end, and the 10 MB budget applies to the largest sample. That also makes the assertion stronger: a result buffered whole shows up mid-stream, not only if something retains it afterwards.

	result
main, whole package, -race -cover -count=8	6 of 8 FAIL (11.4 MB each)
this PR, whole package, -race -cover -count=6, once	ok (384 s); growth 0.06 to 0.10 MB each
this PR, test alone, -race -count=5	5 of 5 pass, 0.06 to 0.10 MB
mutant: promoteToStreaming disabled (if false && ...)	3 of 3 FAIL, 46.3 MB: the property is still guarded

What this test does not cover, stated exactly. It exercises the streaming path (syncLoop = nil); the sync fast path's whole-reply buffering and the fall-through from it to streaming with rows already buffered are not covered. The sync path's memory is NOT bounded by maxDirectResultBytes: appendBuffered checks that cap after it appends the row and cannot stop the poll, so above the cap a reply is still buffered whole before ErrResultTooBig. The reviewer measured a 2M-row reply (about 240 MB on the wire) at 1.1 to 1.2 GB peak heap when the client is slower than the sender (-race and -cover together, which is also what made the 100K-row test flake). That is a separate, pre-existing defect in the driver, not touched here and not hidden by this change (the old 10 MB budget at 100K rows was a streaming expectation the sync path never promised under 64 MiB); it is handled in the separate PR on the sync-path buffering bound.

F2. TestAllocBudgets/Query_1col_1row (alloc_guard_test.go)

The Windows portable-fallback job of #1143 (run 40521342083) logged allocs/op=15 (budget 14). In that log the subtest PASSED: the Portable fallback step does not set TESTING_STRICT_ALLOC_BUDGETS, so the line is informational, and the job failed for another reason (see below). But the 15 is real, and it is not noise:

platform / path	allocs/op
linux/amd64 container, --cpus 2, -count=50	14 in 50 of 50
same, six CPU hogs, -count=30	14 in 30 of 30
same bench with syncLoop = nil (the path of a platform without the sync poll)	15 in 20 of 20
darwin/arm64, native, strict, -count=3	15 in 3 of 3 (the test FAILS strict there today)
Windows portable-fallback job (non-strict)	15

The +1 is the colsCh that simpleQuery allocates before it enqueues when there is no sync loop (if !c.useDirect && c.syncLoop == nil). AllocsPerOp is an integer division (floor), so one stray background allocation cannot turn a 14 into a 15; the 15 is a steady state. The budget was measured on linux only, with zero headroom, which is right for the path it measured.

Fix: the budget follows the path the bench's conn took (conn.syncLoop != nil: 14, else 15), not the OS, with the evidence in a comment. No headroom added: linux stays at 14 (strict, exact), and the other platforms' number is the one measured. With it, the test passes strict on this Mac.

What the Windows job actually failed on

Not Query_1col_1row. The postgres package's output in that job ends at === RUN TestTxDoneAfterFailedCommit followed by FAIL github.com/goceleris/celeris/driver/postgres 6.158s, with no panic, no exit status line and no --- FAIL: the test binary ended there without a message (the test deliberately causes a recovered nil-interface panic in Tx.Rollback, and exists since v1.4.0). I cannot reproduce on Windows. It looks like a runner or process-level event, not an assertion; a rerun is the only evidence available. I did not change anything for it.

Whole package and lint

golang:1.27.3, --cpus 2, -race -cover -count=6 whole ./driver/postgres: ok (above). Strict allocation guards, linux, -count=3: 14 (budget 14) and 0 (budget 0). golangci-lint (CI pin v2.14, linux): 0 issues; go vet on linux, darwin and windows. (A local windows-targeted run shows two findings, rowch_failall_887_test.go:311 errcheck and a nolintlint in conn_netconn.go, both on main and not in this diff.)

Release note

None (tests only).

[coderabbitai]

Caution

Review failed

The pull request is closed.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!