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

Commit b416691

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 b46727e commit b416691

File tree

4 files changed

+44
-29
lines changed

4 files changed

+44
-29
lines changed

src/backend/access/transam/twophase.c

+2-7
Original file line numberDiff line numberDiff line change
@@ -1456,7 +1456,6 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
14561456
RelFileNode *delrels;
14571457
int ndelrels;
14581458
SharedInvalidationMessage *invalmsgs;
1459-
int i;
14601459

14611460
/*
14621461
* Validate the GID, and lock the GXACT to ensure that two backends do not
@@ -1549,13 +1548,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
15491548
delrels = abortrels;
15501549
ndelrels = hdr->nabortrels;
15511550
}
1552-
for (i = 0; i < ndelrels; i++)
1553-
{
1554-
SMgrRelation srel = smgropen(delrels[i], InvalidBackendId);
15551551

1556-
smgrdounlink(srel, false);
1557-
smgrclose(srel);
1558-
}
1552+
/* Make sure files supposed to be dropped are dropped */
1553+
DropRelationFiles(delrels, ndelrels, false);
15591554

15601555
/*
15611556
* Handle cache invalidation messages.

src/backend/access/transam/xact.c

+3-22
Original file line numberDiff line numberDiff line change
@@ -5516,7 +5516,6 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
55165516
RepOriginId origin_id)
55175517
{
55185518
TransactionId max_xid;
5519-
int i;
55205519
TimestampTz commit_time;
55215520

55225521
Assert(TransactionIdIsValid(xid));
@@ -5635,16 +5634,8 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
56355634
*/
56365635
XLogFlush(lsn);
56375636

5638-
for (i = 0; i < parsed->nrels; i++)
5639-
{
5640-
SMgrRelation srel = smgropen(parsed->xnodes[i], InvalidBackendId);
5641-
ForkNumber fork;
5642-
5643-
for (fork = 0; fork <= MAX_FORKNUM; fork++)
5644-
XLogDropRelation(parsed->xnodes[i], fork);
5645-
smgrdounlink(srel, true);
5646-
smgrclose(srel);
5647-
}
5637+
/* Make sure files supposed to be dropped are dropped */
5638+
DropRelationFiles(parsed->xnodes, parsed->nrels, true);
56485639
}
56495640

56505641
/*
@@ -5683,7 +5674,6 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
56835674
static void
56845675
xact_redo_abort(xl_xact_parsed_abort *parsed, TransactionId xid)
56855676
{
5686-
int i;
56875677
TransactionId max_xid;
56885678

56895679
Assert(TransactionIdIsValid(xid));
@@ -5748,16 +5738,7 @@ xact_redo_abort(xl_xact_parsed_abort *parsed, TransactionId xid)
57485738
}
57495739

57505740
/* Make sure files supposed to be dropped are dropped */
5751-
for (i = 0; i < parsed->nrels; i++)
5752-
{
5753-
SMgrRelation srel = smgropen(parsed->xnodes[i], InvalidBackendId);
5754-
ForkNumber fork;
5755-
5756-
for (fork = 0; fork <= MAX_FORKNUM; fork++)
5757-
XLogDropRelation(parsed->xnodes[i], fork);
5758-
smgrdounlink(srel, true);
5759-
smgrclose(srel);
5760-
}
5741+
DropRelationFiles(parsed->xnodes, parsed->nrels, true);
57615742
}
57625743

57635744
void

src/backend/storage/smgr/md.c

+38
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 "pgstat.h"
3132
#include "portability/instr_time.h"
@@ -1703,6 +1704,43 @@ ForgetDatabaseFsyncRequests(Oid dbid)
17031704
}
17041705
}
17051706

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

17071745
/*
17081746
* _fdvec_resize() -- Resize the fork's open segments array

src/include/storage/smgr.h

+1
Original file line numberDiff line numberDiff line change
@@ -143,5 +143,6 @@ extern void RememberFsyncRequest(RelFileNode rnode, ForkNumber forknum,
143143
BlockNumber segno);
144144
extern void ForgetRelationFsyncRequests(RelFileNode rnode, ForkNumber forknum);
145145
extern void ForgetDatabaseFsyncRequests(Oid dbid);
146+
extern void DropRelationFiles(RelFileNode *delrels, int ndelrels, bool isRedo);
146147

147148
#endif /* SMGR_H */

0 commit comments

Comments
 (0)