Skip to content

Commit dd6f261

Browse files
anarazelanarazel
committed
localbuf: Introduce TerminateLocalBufferIO()
Previously TerminateLocalBufferIO() was open-coded in multiple places, which doesn't seem like a great idea. While TerminateLocalBufferIO() currently is rather simple, an upcoming patch requires additional code to be added to TerminateLocalBufferIO(), making this modification particularly worthwhile. For some reason FlushRelationBuffers() previously cleared BM_JUST_DIRTIED, even though that's never set for temporary buffers. This is not carried over as part of this change. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/CAAKRu_b9anbWzEs5AAF9WCvcEVmgz-1AkHSQ-CLLy-p7WHzvFw@mail.gmail.com
1 parent 0762a15 commit dd6f261

File tree

3 files changed

+37
-26
lines changed

3 files changed

+37
-26
lines changed

‎src/backend/storage/buffer/bufmgr.c

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,19 +1072,11 @@ ZeroAndLockBuffer(Buffer buffer, ReadBufferMode mode, bool already_valid)
10721072
if (!isLocalBuf)
10731073
LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_EXCLUSIVE);
10741074

1075+
/* Set BM_VALID, terminate IO, and wake up any waiters */
10751076
if (isLocalBuf)
1076-
{
1077-
/* Only need to adjust flags */
1078-
uint32 buf_state = pg_atomic_read_u32(&bufHdr->state);
1079-
1080-
buf_state |= BM_VALID;
1081-
pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
1082-
}
1077+
TerminateLocalBufferIO(bufHdr, false, BM_VALID);
10831078
else
1084-
{
1085-
/* Set BM_VALID, terminate IO, and wake up any waiters */
10861079
TerminateBufferIO(bufHdr, false, BM_VALID, true);
1087-
}
10881080
}
10891081
else if (!isLocalBuf)
10901082
{
@@ -1554,19 +1546,11 @@ WaitReadBuffers(ReadBuffersOperation *operation)
15541546
relpath(operation->smgr->smgr_rlocator, forknum).str)));
15551547
}
15561548

1557-
/* Terminate I/O and set BM_VALID. */
1549+
/* Set BM_VALID, terminate IO, and wake up any waiters */
15581550
if (persistence == RELPERSISTENCE_TEMP)
1559-
{
1560-
uint32 buf_state = pg_atomic_read_u32(&bufHdr->state);
1561-
1562-
buf_state |= BM_VALID;
1563-
pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
1564-
}
1551+
TerminateLocalBufferIO(bufHdr, false, BM_VALID);
15651552
else
1566-
{
1567-
/* Set BM_VALID, terminate IO, and wake up any waiters */
15681553
TerminateBufferIO(bufHdr, false, BM_VALID, true);
1569-
}
15701554

15711555
/* Report I/Os as completing individually. */
15721556
TRACE_POSTGRESQL_BUFFER_READ_DONE(forknum, io_first_block + j,
@@ -4501,8 +4485,7 @@ FlushRelationBuffers(Relation rel)
45014485
IOCONTEXT_NORMAL, IOOP_WRITE,
45024486
io_start, 1, BLCKSZ);
45034487

4504-
buf_state &= ~(BM_DIRTY | BM_JUST_DIRTIED);
4505-
pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
4488+
TerminateLocalBufferIO(bufHdr, true, 0);
45064489

45074490
pgBufferUsage.local_blks_written++;
45084491

@@ -5589,8 +5572,11 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits,
55895572
buf_state = LockBufHdr(buf);
55905573

55915574
Assert(buf_state & BM_IO_IN_PROGRESS);
5575+
buf_state &= ~BM_IO_IN_PROGRESS;
5576+
5577+
/* Clear earlier errors, if this IO failed, it'll be marked again */
5578+
buf_state &= ~BM_IO_ERROR;
55925579

5593-
buf_state &= ~(BM_IO_IN_PROGRESS | BM_IO_ERROR);
55945580
if (clear_dirty && !(buf_state & BM_JUST_DIRTIED))
55955581
buf_state &= ~(BM_DIRTY | BM_CHECKPOINT_NEEDED);
55965582

‎src/backend/storage/buffer/localbuf.c

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,6 @@ GetLocalVictimBuffer(void)
235235
*/
236236
if (pg_atomic_read_u32(&bufHdr->state) & BM_DIRTY)
237237
{
238-
uint32 buf_state = pg_atomic_read_u32(&bufHdr->state);
239238
instr_time io_start;
240239
SMgrRelation oreln;
241240
Page localpage = (char *) LocalBufHdrGetBlock(bufHdr);
@@ -259,8 +258,7 @@ GetLocalVictimBuffer(void)
259258
IOOP_WRITE, io_start, 1, BLCKSZ);
260259

261260
/* Mark not-dirty now in case we error out below */
262-
buf_state &= ~BM_DIRTY;
263-
pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
261+
TerminateLocalBufferIO(bufHdr, true, 0);
264262

265263
pgBufferUsage.local_blks_written++;
266264
}
@@ -483,6 +481,31 @@ MarkLocalBufferDirty(Buffer buffer)
483481
pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
484482
}
485483

484+
/*
485+
* Like TerminateBufferIO, but for local buffers
486+
*/
487+
void
488+
TerminateLocalBufferIO(BufferDesc *bufHdr, bool clear_dirty, uint32 set_flag_bits)
489+
{
490+
/* Only need to adjust flags */
491+
uint32 buf_state = pg_atomic_read_u32(&bufHdr->state);
492+
493+
/* BM_IO_IN_PROGRESS isn't currently used for local buffers */
494+
495+
/* Clear earlier errors, if this IO failed, it'll be marked again */
496+
buf_state &= ~BM_IO_ERROR;
497+
498+
if (clear_dirty)
499+
buf_state &= ~BM_DIRTY;
500+
501+
buf_state |= set_flag_bits;
502+
pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
503+
504+
/* local buffers don't track IO using resowners */
505+
506+
/* local buffers don't use the IO CV, as no other process can see buffer */
507+
}
508+
486509
/*
487510
* InvalidateLocalBuffer -- mark a local buffer invalid.
488511
*

‎src/include/storage/buf_internals.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,8 @@ extern BlockNumber ExtendBufferedRelLocal(BufferManagerRelation bmr,
471471
Buffer *buffers,
472472
uint32 *extended_by);
473473
extern void MarkLocalBufferDirty(Buffer buffer);
474+
extern void TerminateLocalBufferIO(BufferDesc *bufHdr, bool clear_dirty,
475+
uint32 set_flag_bits);
474476
extern void DropRelationLocalBuffers(RelFileLocator rlocator,
475477
ForkNumber forkNum,
476478
BlockNumber firstDelBlock);

0 commit comments

Comments
 (0)