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

Commit 7da22d8

Browse files
committed
Improve the performance of relation deletes during recovery.
When multiple relations are deleted at the same transaction, the files of those relations are deleted by one call to smgrdounlinkall(), which leads to scan whole shared_buffers only one time. OTOH, previously, during recovery, smgrdounlink() (not smgrdounlinkall()) was called for each file to delete, which led to scan shared_buffers multiple times. Obviously this could cause to increase the WAL replay time very much especially when shared_buffers was huge. To alleviate this situation, this commit changes the recovery so that it also calls smgrdounlinkall() only one time to delete multiple relation files. This is just fix for oversight of commit 279628a, not new feature. So, per discussion on pgsql-hackers, we concluded to backpatch this to all supported versions. Author: Fujii Masao Reviewed-by: Michael Paquier, Andres Freund, Thomas Munro, Kyotaro Horiguchi, Takayuki Tsunakawa Discussion: https://postgr.es/m/CAHGQGwHVQkdfDqtvGVkty+19cQakAydXn1etGND3X0PHbZ3+6w@mail.gmail.com
1 parent d7e5805 commit 7da22d8

File tree

4 files changed

+44
-29
lines changed

4 files changed

+44
-29
lines changed

src/backend/access/transam/twophase.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,7 +1346,6 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
13461346
RelFileNode *delrels;
13471347
int ndelrels;
13481348
SharedInvalidationMessage *invalmsgs;
1349-
int i;
13501349

13511350
/*
13521351
* Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -1438,13 +1437,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
14381437
delrels = abortrels;
14391438
ndelrels = hdr->nabortrels;
14401439
}
1441-
for (i = 0; i < ndelrels; i++)
1442-
{
1443-
SMgrRelation srel = smgropen(delrels[i], InvalidBackendId);
14441440

1445-
smgrdounlink(srel, false);
1446-
smgrclose(srel);
1447-
}
1441+
/* Make sure files supposed to be dropped are dropped */
1442+
DropRelationFiles(delrels, ndelrels, false);
14481443

14491444
/*
14501445
* Handle cache invalidation messages.

src/backend/access/transam/xact.c

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5349,7 +5349,6 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
53495349
RepOriginId origin_id)
53505350
{
53515351
TransactionId max_xid;
5352-
int i;
53535352
TimestampTz commit_time;
53545353

53555354
max_xid = TransactionIdLatest(xid, parsed->nsubxacts, parsed->subxacts);
@@ -5467,16 +5466,8 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
54675466
*/
54685467
XLogFlush(lsn);
54695468

5470-
for (i = 0; i < parsed->nrels; i++)
5471-
{
5472-
SMgrRelation srel = smgropen(parsed->xnodes[i], InvalidBackendId);
5473-
ForkNumber fork;
5474-
5475-
for (fork = 0; fork <= MAX_FORKNUM; fork++)
5476-
XLogDropRelation(parsed->xnodes[i], fork);
5477-
smgrdounlink(srel, true);
5478-
smgrclose(srel);
5479-
}
5469+
/* Make sure files supposed to be dropped are dropped */
5470+
DropRelationFiles(parsed->xnodes, parsed->nrels, true);
54805471
}
54815472

54825473
/*
@@ -5515,7 +5506,6 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
55155506
static void
55165507
xact_redo_abort(xl_xact_parsed_abort *parsed, TransactionId xid)
55175508
{
5518-
int i;
55195509
TransactionId max_xid;
55205510

55215511
/*
@@ -5577,16 +5567,7 @@ xact_redo_abort(xl_xact_parsed_abort *parsed, TransactionId xid)
55775567
}
55785568

55795569
/* Make sure files supposed to be dropped are dropped */
5580-
for (i = 0; i < parsed->nrels; i++)
5581-
{
5582-
SMgrRelation srel = smgropen(parsed->xnodes[i], InvalidBackendId);
5583-
ForkNumber fork;
5584-
5585-
for (fork = 0; fork <= MAX_FORKNUM; fork++)
5586-
XLogDropRelation(parsed->xnodes[i], fork);
5587-
smgrdounlink(srel, true);
5588-
smgrclose(srel);
5589-
}
5570+
DropRelationFiles(parsed->xnodes, parsed->nrels, true);
55905571
}
55915572

55925573
void

src/backend/storage/smgr/md.c

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <sys/file.h>
2727

2828
#include "miscadmin.h"
29+
#include "access/xlogutils.h"
2930
#include "access/xlog.h"
3031
#include "catalog/catalog.h"
3132
#include "portability/instr_time.h"
@@ -1701,6 +1702,43 @@ ForgetDatabaseFsyncRequests(Oid dbid)
17011702
}
17021703
}
17031704

1705+
/*
1706+
* DropRelationFiles -- drop files of all given relations
1707+
*/
1708+
void
1709+
DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo)
1710+
{
1711+
SMgrRelation *srels;
1712+
int i;
1713+
1714+
srels = palloc(sizeof(SMgrRelation) * ndelrels);
1715+
for (i = 0; i < ndelrels; i++)
1716+
{
1717+
SMgrRelation srel = smgropen(delrels[i], InvalidBackendId);
1718+
1719+
if (isRedo)
1720+
{
1721+
ForkNumber fork;
1722+
1723+
for (fork = 0; fork <= MAX_FORKNUM; fork++)
1724+
XLogDropRelation(delrels[i], fork);
1725+
}
1726+
srels[i] = srel;
1727+
}
1728+
1729+
smgrdounlinkall(srels, ndelrels, isRedo);
1730+
1731+
/*
1732+
* Call smgrclose() in reverse order as when smgropen() is called.
1733+
* This trick enables remove_from_unowned_list() in smgrclose()
1734+
* to search the SMgrRelation from the unowned list,
1735+
* with O(1) performance.
1736+
*/
1737+
for (i = ndelrels - 1; i >= 0; i--)
1738+
smgrclose(srels[i]);
1739+
pfree(srels);
1740+
}
1741+
17041742

17051743
/*
17061744
* _fdvec_alloc() -- Make a MdfdVec object.

src/include/storage/smgr.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ extern void RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum,
139139
BlockNumber segno);
140140
extern void ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum);
141141
extern void ForgetDatabaseFsyncRequests(Oid dbid);
142+
extern void DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo);
142143

143144
/* smgrtype.c */
144145
extern Datum smgrout(PG_FUNCTION_ARGS);

0 commit comments

Comments
 (0)