Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit9110d81

Browse files
committed
Fix rare bug in read_stream.c's split IO handling.
The internal queue of buffers could become corrupted in a rare edge casethat failed to invalidate an entry, causing a stale buffer to be"forwarded" to StartReadBuffers(). This is a simple fix for theimmediate problem.A small API change might be able to remove this and related fragilityentirely, but that will have to wait a bit.Defect in commited0b87c.Bug: 19006Backpatch-through: 18Reported-by: Alexander Lakhin <exclusion@gmail.com>Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>Reviewed-by: Michael Paquier <michael@paquier.xyz>Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>Discussion:https://postgr.es/m/19006-80fcaaf69000377e%40postgresql.org
1 parent762fed9 commit9110d81

File tree

1 file changed

+34
-0
lines changed

1 file changed

+34
-0
lines changed

‎src/backend/storage/aio/read_stream.c‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,33 @@ read_stream_start_pending_read(ReadStream *stream)
247247
Assert(stream->pinned_buffers+stream->pending_read_nblocks <=
248248
stream->max_pinned_buffers);
249249

250+
#ifdefUSE_ASSERT_CHECKING
250251
/* We had better not be overwriting an existing pinned buffer. */
251252
if (stream->pinned_buffers>0)
252253
Assert(stream->next_buffer_index!=stream->oldest_buffer_index);
253254
else
254255
Assert(stream->next_buffer_index==stream->oldest_buffer_index);
255256

257+
/*
258+
* Pinned buffers forwarded by a preceding StartReadBuffers() call that
259+
* had to split the operation should match the leading blocks of this
260+
* following StartReadBuffers() call.
261+
*/
262+
Assert(stream->forwarded_buffers <=stream->pending_read_nblocks);
263+
for (inti=0;i<stream->forwarded_buffers;++i)
264+
Assert(BufferGetBlockNumber(stream->buffers[stream->next_buffer_index+i])==
265+
stream->pending_read_blocknum+i);
266+
267+
/*
268+
* Check that we've cleared the queue/overflow entries corresponding to
269+
* the rest of the blocks covered by this read, unless it's the first go
270+
* around and we haven't even initialized them yet.
271+
*/
272+
for (inti=stream->forwarded_buffers;i<stream->pending_read_nblocks;++i)
273+
Assert(stream->next_buffer_index+i >=stream->initialized_buffers||
274+
stream->buffers[stream->next_buffer_index+i]==InvalidBuffer);
275+
#endif
276+
256277
/* Do we need to issue read-ahead advice? */
257278
flags=stream->read_buffers_flags;
258279
if (stream->advice_enabled)
@@ -979,6 +1000,19 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data)
9791000
stream->pending_read_nblocks==0&&
9801001
stream->per_buffer_data_size==0)
9811002
{
1003+
/*
1004+
* The fast path spins on one buffer entry repeatedly instead of
1005+
* rotating through the whole queue and clearing the entries behind
1006+
* it. If the buffer it starts with happened to be forwarded between
1007+
* StartReadBuffers() calls and also wrapped around the circular queue
1008+
* partway through, then a copy also exists in the overflow zone, and
1009+
* it won't clear it out as the regular path would. Do that now, so
1010+
* it doesn't need code for that.
1011+
*/
1012+
if (stream->oldest_buffer_index<stream->io_combine_limit-1)
1013+
stream->buffers[stream->queue_size+stream->oldest_buffer_index]=
1014+
InvalidBuffer;
1015+
9821016
stream->fast_path= true;
9831017
}
9841018
#endif

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp