[FumingPower3925]

driver/postgres: a reply buffered on the sync path / direct mode is bounded by the cap, and grows by chunks instead of holding 5x its data. A product defect found through TestStreamingLarge, separate from the test PR #1144. Refs #1147, Refs #1104

SEMANTICS NOTE (read first)

This changes what a caller sees for a reply over maxDirectResultBytes (64 MiB of payload): it used to get ALL the rows and then ErrResultTooBig at the end of the iteration; now it gets the rows buffered up to the cap and then ErrResultTooBig. The error value and the threshold (payload bytes over the cap) are unchanged, and a reply that fits is returned whole as before. A caller that ignored rows.Err() and relied on getting everything from an over-cap query would now see a truncated result with the error it was not checking; a single row over the cap fails the query itself (main returned that row, then the error). No API change. The error value and the threshold are unchanged; the decision to take this behaviour is the maintainer's.

The defect

On the sync fast path (pollSync -> onRecv -> appendBuffered) and in direct mode a reply is buffered whole (syncMode: the caller's goroutine is the dispatcher, so it cannot promote to streaming). Two problems:

1. appendBuffered appended the row and only then tested the cap, and the poll cannot be stopped from there, so a reply above the cap was buffered whole and the error came at the end: the cap was a flag, not a bound.
2. The slab and the row-field array grew by copy with 1.25x steps, while the row views already handed out kept pointing into the old arrays: every earlier generation stayed reachable, 1 / (1 - 1/1.25) = 5 times the data live.

Measured (linux/arm64 docker, --cpus 2, the fake PG of the package tests): a 2M-row reply (about 86 MB payload, direct mode) peaked at 731 to 1097 MB of live heap; with only the cap moved before the append, 888 to 923 MB (the cap alone does not bound it); 400,000 rows of 64 bytes (24 MB) held 199 MB live. When it happens on the sync fast path: only when the client is not faster than the sender (instrumented, loaded, or a fast server); on an idle uninstrumented box the poll hits EAGAIN and the request streams. Direct mode always.

The fix

• Commit 2: the cap is checked before the append (slabBytes + row > maxDirectResultBytes); a row that does not fit is not appended, the request fails with ErrResultTooBig, and the rest of the reply is read and dropped (the wire stays aligned). A row that would fit after a failed one is not buffered either (reply order, no hole).
• Commit 4: a full chunk is not copied; the next is started (doubling to 64K, then fixed), and slabBytes counts the payload over all chunks. The earlier views stay valid exactly as they did after a copy. Live heap = the data plus 24 bytes of field view and 24 of row view per row, plus at most one row's worth of unused tail per chunk (worst case about twice the data, for rows just over half a chunk). growCap had no other user and is gone.

Residual, not addressed: the cap counts payload bytes, so many NULL or tiny fields cost about 24 bytes per field and per row beyond it.

• Commit 6 (found by review): the drop after the cap sat inside if syncMode, and the sync fast path clears syncMode when its poll returns before the reply is done (EAGAIN), so the rest of the reply, dispatched by the event loop, was buffered again and promoted to streaming behind the rows already there: a gap and io.EOF, silent data loss (1124 of 1200 rows, gap after row 1023, in simpleQuery and doExtendedQuery; main returned the whole result there, with the error lost). The guard is outside if syncMode now: after ErrResultTooBig appendBuffered returns false in every mode, which also keeps promoteToStreaming from running, so the caller wakes on doneCh with the buffered rows and the error. The payload size is summed once and passed to the append.

Every path by which appendBuffered can run after the cap tripped

appendBuffered is reached only from OnRow, called by the simple-query and extended-query parsers for a DataRow (nothing else, COPY included, calls it).

path	cap	after the cap tripped
direct mode (driveDirect, syncMode pinned)	applies	rows dropped (TestDirectModeResultAboveTheCapIsBounded1104)
sync fast path, reply done inside the poll	applies	rows dropped, rows up to the cap then the error (simpleQuery/doExtendedQuery buffered-rows branch)
sync fast path, poll returns before the reply is done, event loop dispatches the rest (syncMode false)	applied while the poll ran	rows dropped (the guard is mode-independent now): TestSyncFallThroughAfterTheCapKeepsOrderAndTheError1104, both query kinds; red on 202f199 10 of 10
async / non-Linux loop / WithEngine async (syncMode never set)	never applies	the cap never trips, lazy streaming
promoted (streaming) request	OnRow sends to rowCh before appendBuffered	not reached
extended query	same OnRow	same as simple (the test covers it)
Exec (simpleExec, doExtendedExec, no-tag)	syncMode never set, no cap	unchanged: rows buffered then dropped, now chunked (1x, not 5x)
a single row over the cap, as the first row	cap trips on row 1, nothing buffered	the query itself fails: QueryContext returns ErrResultTooBig (no row to hand back); main returned that one row and then the error from Next
COPY	onCopyRow, not OnRow	not reached
several statements in one simple Query	slabBytes accumulates over the request, as len(rowSlab) did	unchanged

Evidence

golang:1.27.3, --cpus 2, 8 MiB memlock.

	main	this PR
-race -count=10 of the new tests (commit 1 + 3 tests on main's code)	10 of 10 FAIL each of cap-in-request, cap end to end (direct mode: 85,536 rows read of a cap of 65,536 at 1 KiB), and live heap (199 MB for 24 MB of rows); the two "fits" cases pass	10 of 10 pass, all
2M-row reply, peak live heap	731 to 1097 MB	205 to 206 MB
TestAllocBudgets strict, linux, -count=5	14	14 (budget 14), 0 (budget 0)
whole ./driver/postgres, -race -cover -count=1, once		ok (65 s)

Mutants, each on this PR's code, the named test then fails (the guard put back inside if syncMode: the fall-through test red, 3 of 3): no cap check: cap tests red; no drop after the error: the order test red (a 512-byte row after a failed 2 KiB one was buffered); slab grows by copy: live-heap test red (and the same for the row fields).

Small-result path, -count=7 medians, two rounds, docker --cpus 2:

benchmark	main	this PR
end to end QueryContext, 1 col 1 row	31.7 us, 14 allocs, 204 B	34.1 us, 14 allocs, 206 B (round medians 30.9/34.4/30.7 vs 35.1/30.5/36.0: the run-to-run noise of a loopback round trip is larger than the difference)
OnRow on a pooled request, 1 row 1 field	368 ns, 10 allocs	393 ns, 10 allocs (+25 ns: the size sum of the cap check and one counter)
OnRow x100 rows x3 fields	5.1 us, 38.6 KB, 24 allocs	3.8 us, 23.0 KB, 21 allocs (-25%)
OnRow x10,000 rows x3 fields, FRESH request	1.35 ms, 6.0 MB, 73 allocs	0.38 ms, 1.9 MB, 44 allocs (-72%, -69% B: a cold request only, see the pooled case below)
OnRow x10,000 rows x3 fields, WARM pooled request (reviewer's measurement)	0 B/op, 0 allocs: the release keeps the whole grown slab and field array, so the next reply of the same size reuses them	459 KB/op, 7 allocs: only the last chunk is kept, so every reply over 64 KiB of payload (or 64K fields) allocates its chunks again; ns/op within noise (232 to 325 us vs 272 us)
end to end, 10 / 100 / 1000 rows	137 / 325 / 1899 us	129 / 280 / 1933 us

Re-measured after the single size sum (5 interleaved rounds of 3 runs, medians, docker --cpus 2, slabBytes last in the struct): OnRow 1 row 1 field 354 -> 377 ns (+6%, 10 allocs both, struct 568 -> 576 bytes, same 576 size class), end to end QueryContext 1 col 1 row 31.8 -> 33.2 us (+4.6%, 14 allocs both, 205 / 204 B), which is inside the run-to-run noise of a loopback round trip (earlier rounds swung from -13% to +16%). The pooled-request regression, stated plainly. The "-69% B/op" above holds for a cold request. A service that repeatedly fetches mid-size results (about 1,000 to 10,000 rows) used to reuse the grown slab from the request pool at 0 B/op and now allocates about the payload per query: more GC work, no measured latency change. It is the other side of bounding what the pool retains (main kept a slab of up to 64 MiB, and 5 times its data in generations, per pooled request) and of the 5x fix; I did not change the code. A possible follow-up is to keep up to N chunks (for example 256 KiB) on release.

Measurement caveat: the reviewer measured OnRow 1x1 at 26 to 27 ns on main, on the earlier head and on this one; my +23 ns (354 -> 377 ns, the benchmark including the pool round trip) did not reproduce under his load. Differences under about 10% on this loaded Mac are noise, and I draw no conclusion from either figure beyond "no measurable change on the small-result path".

So: the small-result path is unchanged within noise (allocs identical), and results of hundreds of rows and up get cheaper, with no copy of the growing slab.

Lint (CI pin v2.14): 0 issues; go vet on linux, darwin and windows.

Release note

driver/postgres: a reply over the 64 MiB buffer cap (direct mode, and the sync fast path while its poll is reading) now returns the rows that fit under the cap and then ErrResultTooBig from rows.Next / rows.Err(), instead of every row and then the error (a caller that ignores rows.Err() sees a truncated result; a single row over the cap fails the query itself); the rest of the reply is discarded and the connection stays usable. Fixed: a buffered reply no longer holds about five times its payload in live heap (2M rows, measured two ways: 731 to 1097 MB -> 205 MB here, 741 MB -> 145 MB by the reviewer). Trade-off: a warm pooled request no longer keeps a grown slab, so repeated replies over 64 KiB of payload allocate their chunks each time.

[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!