Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3072b7b

Browse files
committed
Send only one FORGET_RELATION_FSYNC request when dropping a relation.
We were sending one per fork, but a little bit of refactoring allows us to send just one request with forknum == InvalidForkNumber. This not only reduces pressure on the shared-memory request queue, but saves repeated traversals of the checkpointer's hash table.
1 parent a7a4add commit 3072b7b

File tree

2 files changed

+33
-9
lines changed

2 files changed

+33
-9
lines changed

src/backend/storage/smgr/md.c

+32-7
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ typedef enum /* behavior for mdopen & _mdfd_getseg */
166166
} ExtensionBehavior;
167167

168168
/* local routines */
169+
static void mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum,
170+
bool isRedo);
169171
static MdfdVec *mdopen(SMgrRelation reln, ForkNumber forknum,
170172
ExtensionBehavior behavior);
171173
static void register_dirty_segment(SMgrRelation reln, ForkNumber forknum,
@@ -308,6 +310,9 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
308310
* Note that we're passed a RelFileNodeBackend --- by the time this is called,
309311
* there won't be an SMgrRelation hashtable entry anymore.
310312
*
313+
* forkNum can be a fork number to delete a specific fork, or InvalidForkNumber
314+
* to delete all forks.
315+
*
311316
* For regular relations, we don't unlink the first segment file of the rel,
312317
* but just truncate it to zero length, and record a request to unlink it after
313318
* the next checkpoint. Additional segments can be unlinked immediately,
@@ -349,17 +354,32 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
349354
void
350355
mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
351356
{
352-
char *path;
353-
int ret;
354-
355357
/*
356358
* We have to clean out any pending fsync requests for the doomed
357359
* relation, else the next mdsync() will fail. There can't be any such
358-
* requests for a temp relation, though.
360+
* requests for a temp relation, though. We can send just one request
361+
* even when deleting multiple forks, since the fsync queuing code accepts
362+
* the "InvalidForkNumber = all forks" convention.
359363
*/
360364
if (!RelFileNodeBackendIsTemp(rnode))
361365
ForgetRelationFsyncRequests(rnode.node, forkNum);
362366

367+
/* Now do the per-fork work */
368+
if (forkNum == InvalidForkNumber)
369+
{
370+
for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
371+
mdunlinkfork(rnode, forkNum, isRedo);
372+
}
373+
else
374+
mdunlinkfork(rnode, forkNum, isRedo);
375+
}
376+
377+
static void
378+
mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
379+
{
380+
char *path;
381+
int ret;
382+
363383
path = relpath(rnode, forkNum);
364384

365385
/*
@@ -1340,7 +1360,8 @@ register_unlink(RelFileNodeBackend rnode)
13401360
* The range of possible segment numbers is way less than the range of
13411361
* BlockNumber, so we can reserve high values of segno for special purposes.
13421362
* We define three:
1343-
* - FORGET_RELATION_FSYNC means to cancel pending fsyncs for a relation
1363+
* - FORGET_RELATION_FSYNC means to cancel pending fsyncs for a relation,
1364+
* either for one fork, or all forks if forknum is InvalidForkNumber
13441365
* - FORGET_DATABASE_FSYNC means to cancel pending fsyncs for a whole database
13451366
* - UNLINK_RELATION_REQUEST is a request to delete the file after the next
13461367
* checkpoint.
@@ -1356,15 +1377,16 @@ RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
13561377

13571378
if (segno == FORGET_RELATION_FSYNC)
13581379
{
1359-
/* Remove any pending requests for the entire relation */
1380+
/* Remove any pending requests for the relation (one or all forks) */
13601381
HASH_SEQ_STATUS hstat;
13611382
PendingOperationEntry *entry;
13621383

13631384
hash_seq_init(&hstat, pendingOpsTable);
13641385
while ((entry = (PendingOperationEntry *) hash_seq_search(&hstat)) != NULL)
13651386
{
13661387
if (RelFileNodeEquals(entry->tag.rnode, rnode) &&
1367-
entry->tag.forknum == forknum)
1388+
(entry->tag.forknum == forknum ||
1389+
forknum == InvalidForkNumber))
13681390
{
13691391
/* Okay, cancel this entry */
13701392
entry->canceled = true;
@@ -1466,6 +1488,9 @@ RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
14661488

14671489
/*
14681490
* ForgetRelationFsyncRequests -- forget any fsyncs for a relation fork
1491+
*
1492+
* forknum == InvalidForkNumber means all forks, although this code doesn't
1493+
* actually know that, since it's just forwarding the request elsewhere.
14691494
*/
14701495
void
14711496
ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum)

src/backend/storage/smgr/smgr.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,7 @@ smgrdounlink(SMgrRelation reln, bool isRedo)
381381
* ERROR, because we've already decided to commit or abort the current
382382
* xact.
383383
*/
384-
for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
385-
(*(smgrsw[which].smgr_unlink)) (rnode, forknum, isRedo);
384+
(*(smgrsw[which].smgr_unlink)) (rnode, InvalidForkNumber, isRedo);
386385
}
387386

388387
/*

0 commit comments

Comments
 (0)