Fix if/while thinko in read_stream.c edge case.

macdice · macdice · commit 158f58192368 · 2024-04-07T14:51:33.000+12:00
When we determine that a wanted block can't be combined with the current pending read, it's time to start that read to get it out of the way. An "if" in that code path should have been a "while", because it might take more than one go in case of partial reads. This was only broken for smaller ranges, as the more common case of io_combine_limit-sized ranges is handled earlier in the code and knows how to loop, hiding the bug for a while. Discovered while testing large parallel sequential scans of partially cached tables. The ramp-up-and-down block allocator for parallel scans could hit the problem case and skip some blocks near the end that should have been streamed. Defect in commit b5a9b18. Discussion: https://postgr.es/m/CA%2BhUKG%2Bh8Whpv0YsJqjMVkjYX%2B80fTVc6oi-V%2BzxJvykLpLHYQ%40mail.gmail.com
diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c
@@ -363,7 +363,7 @@ read_stream_look_ahead(ReadStream *stream, bool suppress_advice)
 		}
 
 		/* We have to start the pending read before we can build another. */
-		if (stream->pending_read_nblocks > 0)
+		while (stream->pending_read_nblocks > 0)
 		{
 			read_stream_start_pending_read(stream, suppress_advice);
 			suppress_advice = false;

Original file line number	Diff line number	Diff line change
`@@ -363,7 +363,7 @@ read_stream_look_ahead(ReadStream *stream, bool suppress_advice)`
`363`	`363`	`}`
`364`	`364`
`365`	`365`	`/* We have to start the pending read before we can build another. */`
`366`		`- if (stream->pending_read_nblocks > 0)`
	`366`	`+ while (stream->pending_read_nblocks > 0)`
`367`	`367`	`{`
`368`	`368`	`read_stream_start_pending_read(stream, suppress_advice);`
`369`	`369`	`suppress_advice = false;`