public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v11 5/6] Move removal of old serialized snapshots to custodian. 20+ messages / 2 participants [nested] [flat]
* [PATCH v11 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f32b2124e6..677cead44e 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7028,10 +7028,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1d8ebb4c0d..d3bbc59389 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2027,14 +2027,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index f126ff2e08..4877afb1bd 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --envbJBWh7q8WU6mo Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0006-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v5 5/8] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 2 -- src/backend/postmaster/custodian.c | 11 +++++++++++ src/backend/replication/logical/snapbuild.c | 13 +++++++------ src/include/replication/snapbuild.h | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ce78ac413e..c4a80ea82a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -79,7 +79,6 @@ #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6807,7 +6806,6 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 5bad0af474..8591c5db9b 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -40,6 +40,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -208,6 +209,16 @@ CustodianMain(void) */ RemovePgTempFiles(false, false); + /* + * Remove serialized snapshots that are no longer required by any + * logical replication slot. + * + * It is not important for these to be removed in single-user mode, so + * we don't need any extra handling outside of the custodian process for + * this. + */ + RemoveOldSerializedSnapshots(); + /* Calculate how long to sleep */ end_time = (pg_time_t) time(NULL); elapsed_secs = end_time - start_time; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 83fca8a77d..466a6478f3 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -125,6 +125,7 @@ #include "access/xact.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/interrupt.h" #include "replication/logical.h" #include "replication/reorderbuffer.h" #include "replication/snapbuild.h" @@ -1912,14 +1913,13 @@ snapshot_not_interesting: /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; @@ -1942,7 +1942,8 @@ CheckPointSnapBuild(void) cutoff = redo; snap_dir = AllocateDir("pg_logical/snapshots"); - while ((snap_de = ReadDir(snap_dir, "pg_logical/snapshots")) != NULL) + while (!ShutdownRequestPending && + (snap_de = ReadDir(snap_dir, "pg_logical/snapshots")) != NULL) { uint32 hi; uint32 lo; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index d179251aad..55a2beb434 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --2oS5YaxWCcQjTEyO Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0006-Move-removal-of-old-logical-rewrite-mapping-files.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v10 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 7a710e6490..cbe86c6822 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6848,10 +6848,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1d8ebb4c0d..d3bbc59389 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2027,14 +2027,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index e6adea24f2..e1de013ece 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --+QahgC5+KEYLbs62 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0006-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v18 2/4] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- contrib/test_decoding/expected/rewrite.out | 21 +++++++++++++++++++++ contrib/test_decoding/sql/rewrite.sql | 17 +++++++++++++++++ src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 7 files changed, 50 insertions(+), 9 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index b30999c436..8b97f15f6f 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -162,3 +162,24 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index 62dead3a9b..d268fa559a 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -105,3 +105,20 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; + +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..c153c32a77 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7001,10 +7001,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index e5af958999..9382d524a6 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index beddcbcdea..e7c4f69b42 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2036,14 +2036,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --LZvS9be/3tNcYl/X Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v18-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v19 2/4] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- contrib/test_decoding/expected/rewrite.out | 21 +++++++++++++++++++++ contrib/test_decoding/sql/rewrite.sql | 17 +++++++++++++++++ src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 7 files changed, 50 insertions(+), 9 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index b30999c436..8b97f15f6f 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -162,3 +162,24 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index 62dead3a9b..d268fa559a 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -105,3 +105,20 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; + +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index fb4c860bde..382e59f723 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6997,10 +6997,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 98bb9efcfd..4e0ce1f7b3 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 829c568112..6b403a2bb4 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2036,14 +2036,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index f49b941b53..5f1ba3842c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --x+6KMIRAuhnl3hBn Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v19-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v20 2/4] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- contrib/test_decoding/expected/rewrite.out | 21 +++++++++++++++++++++ contrib/test_decoding/sql/rewrite.sql | 17 +++++++++++++++++ src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 7 files changed, 50 insertions(+), 9 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index b30999c436..8b97f15f6f 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -162,3 +162,24 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index 62dead3a9b..d268fa559a 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -105,3 +105,20 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; + +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f9f0f6db8d..7da9461048 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6994,10 +6994,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 98bb9efcfd..4e0ce1f7b3 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 62542827e4..f940bb5930 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2036,14 +2036,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index f49b941b53..5f1ba3842c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --CE+1k2dSO48ffgeK Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v20-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v10 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 7a710e6490..cbe86c6822 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6848,10 +6848,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1d8ebb4c0d..d3bbc59389 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2027,14 +2027,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index e6adea24f2..e1de013ece 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --+QahgC5+KEYLbs62 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0006-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v6 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 12 ++++++++++++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 3 ++- src/include/replication/snapbuild.h | 2 +- 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 8764084e21..621bda0844 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -75,13 +75,13 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/basebackup.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6840,10 +6840,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index a0ec94ea5c..861de882c6 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -31,6 +31,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -210,6 +211,17 @@ CustodianMain(void) if (flags & CUSTODIAN_REMOVE_TEMP_FILES) RemovePgTempFiles(false, false); + /* + * Remove serialized snapshots that are no longer required by any + * logical replication slot. + * + * It is not important for these to be removed in single-user mode, so + * we don't need any extra handling outside of the custodian process for + * this. + */ + if (flags & CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS) + RemoveOldSerializedSnapshots(); + /* Calculate how long to sleep */ end_time = (pg_time_t) time(NULL); elapsed_secs = end_time - start_time; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1119a12db9..42eb064bd8 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1911,14 +1911,13 @@ snapshot_not_interesting: /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index f6dcd9ddef..769c07f2c9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,6 +18,7 @@ extern void CustodianShmemInit(void); extern void RequestCustodian(int flags); /* flags for RequestCustodian() */ -#define CUSTODIAN_REMOVE_TEMP_FILES 0x0001 +#define CUSTODIAN_REMOVE_TEMP_FILES 0x0001 +#define CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS 0x0002 #endif /* _CUSTODIAN_H */ diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index d179251aad..55a2beb434 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --liOOAslEiF7prFVr Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0006-Move-removal-of-old-logical-rewrite-mapping-files.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v7 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 1b2f240228..f08a18d273 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -75,13 +75,13 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/basebackup.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6837,10 +6837,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 73c0f15214..b945744e9c 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1911,14 +1911,13 @@ snapshot_not_interesting: /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index d179251aad..55a2beb434 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --/04w6evG8XlLl3ft Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0006-Move-removal-of-old-logical-rewrite-mapping-files.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v8 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 9cedd6876f..72645f1fe6 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6846,10 +6846,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1ff2c12240..abafdb52b2 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2014,14 +2014,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index e6adea24f2..e1de013ece 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --X1bOJ3K7DJ5YkBrT Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0006-Move-removal-of-old-logical-rewrite-mapping-files.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v9 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 87b243e0d4..88d10874e2 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -6842,10 +6842,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 1ff2c12240..abafdb52b2 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2014,14 +2014,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index e6adea24f2..e1de013ece 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --y0ulUmNC+osPPQO6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0006-Move-removal-of-old-logical-rewrite-mapping-files.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v12 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index be54c23187..03fdcf2c07 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7024,10 +7024,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 5006a5c464..a161cf3995 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2030,14 +2030,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --UlVJffcvxoiEqYs2 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v12-0006-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v13 5/6] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 8 ++++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 1 + src/include/replication/snapbuild.h | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..4991c10f86 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7001,10 +7001,14 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + !IsUnderPostmaster, + (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index fe1f48844e..855a756ca0 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -71,6 +72,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_TEMP_FILES, RemovePgTempFiles, NULL}, + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index a1fd1d92d6..f957b9aa49 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2037,14 +2037,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 80890ceadd..37334941cc 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -19,6 +19,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --CE+1k2dSO48ffgeK Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v13-0006-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v14 2/3] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..c153c32a77 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7001,10 +7001,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index a94381bc21..d0fd955d4b 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index a1fd1d92d6..f957b9aa49 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2037,14 +2037,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --BXVAT5kNtrzKuDFl Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v14-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v15 2/4] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..c153c32a77 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7001,10 +7001,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index a94381bc21..d0fd955d4b 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index a1fd1d92d6..f957b9aa49 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2037,14 +2037,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --HcAYCG3uE/tztfnV Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v15-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v16 2/4] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- contrib/test_decoding/expected/spill.out | 21 +++++++++++++++++++++ contrib/test_decoding/sql/spill.sql | 17 +++++++++++++++++ src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 7 files changed, 50 insertions(+), 9 deletions(-) diff --git a/contrib/test_decoding/expected/spill.out b/contrib/test_decoding/expected/spill.out index 10734bdb6a..75acbd5d5c 100644 --- a/contrib/test_decoding/expected/spill.out +++ b/contrib/test_decoding/expected/spill.out @@ -248,6 +248,27 @@ GROUP BY 1 ORDER BY 1; (2 rows) DROP TABLE spill_test; +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); + ?column? +---------- + t +(1 row) + SELECT pg_drop_replication_slot('regression_slot'); pg_drop_replication_slot -------------------------- diff --git a/contrib/test_decoding/sql/spill.sql b/contrib/test_decoding/sql/spill.sql index e638cacd3f..94d522f548 100644 --- a/contrib/test_decoding/sql/spill.sql +++ b/contrib/test_decoding/sql/spill.sql @@ -176,4 +176,21 @@ GROUP BY 1 ORDER BY 1; DROP TABLE spill_test; +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); + SELECT pg_drop_replication_slot('regression_slot'); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..c153c32a77 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7001,10 +7001,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index a94381bc21..d0fd955d4b 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index beddcbcdea..e7c4f69b42 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2036,14 +2036,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --lrZ03NoBR/3+SXJZ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v16-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v17 2/4] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- contrib/test_decoding/expected/rewrite.out | 21 +++++++++++++++++++++ contrib/test_decoding/sql/rewrite.sql | 17 +++++++++++++++++ src/backend/access/transam/xlog.c | 6 ++++-- src/backend/postmaster/custodian.c | 2 ++ src/backend/replication/logical/snapbuild.c | 9 ++++----- src/include/postmaster/custodian.h | 2 +- src/include/replication/snapbuild.h | 2 +- 7 files changed, 50 insertions(+), 9 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index b30999c436..8b97f15f6f 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -162,3 +162,24 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index 62dead3a9b..d268fa559a 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -105,3 +105,20 @@ DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); DROP FUNCTION exec(text); DROP ROLE regress_justforcomments; + +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + snaps_removed bool; + loops int := 0; +BEGIN + LOOP + snaps_removed := count(*) = 0 FROM pg_ls_logicalsnapdir(); + IF snaps_removed OR loops > 120 * 100 THEN EXIT; END IF; + PERFORM pg_sleep(0.01); + loops := loops + 1; + END LOOP; +END +$$; +SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..c153c32a77 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -76,12 +76,12 @@ #include "port/atomics.h" #include "port/pg_iovec.h" #include "postmaster/bgwriter.h" +#include "postmaster/custodian.h" #include "postmaster/startup.h" #include "postmaster/walwriter.h" #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -7001,10 +7001,12 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); + /* tasks offloaded to custodian */ + RequestCustodian(CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, (Datum) 0); + /* Write out all dirty data in SLRUs and the main buffer pool */ TRACE_POSTGRESQL_BUFFER_CHECKPOINT_START(flags); CheckpointStats.ckpt_write_t = GetCurrentTimestamp(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index a94381bc21..d0fd955d4b 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -25,6 +25,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/fd.h" @@ -70,6 +71,7 @@ struct cust_task_funcs_entry * whether the task is already enqueued. */ static const struct cust_task_funcs_entry cust_task_functions[] = { + {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index beddcbcdea..e7c4f69b42 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -2036,14 +2036,13 @@ SnapBuildRestoreContents(int fd, char *dest, Size size, const char *path) /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 73d0bc5f02..ab6d4283b9 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -18,7 +18,7 @@ */ typedef enum CustodianTask { - FAKE_TASK, /* placeholder until we have a real task */ + CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index 2a697e57c3..9eba403e0c 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *reorder, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --ew6BAiZeqk4r7MaW Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v17-0003-Move-removal-of-old-logical-rewrite-mapping-file.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* [PATCH v4 5/8] Move removal of old serialized snapshots to custodian. @ 2021-12-06 06:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Nathan Bossart @ 2021-12-06 06:02 UTC (permalink / raw) This was only done during checkpoints because it was a convenient place to put it. However, if there are many snapshots to remove, it can significantly extend checkpoint time. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/transam/xlog.c | 2 -- src/backend/postmaster/custodian.c | 11 +++++++++++ src/backend/replication/logical/snapbuild.c | 13 +++++++------ src/include/replication/snapbuild.h | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 958220c495..369e0711f1 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -56,7 +56,6 @@ #include "replication/logical.h" #include "replication/origin.h" #include "replication/slot.h" -#include "replication/snapbuild.h" #include "replication/walreceiver.h" #include "replication/walsender.h" #include "storage/bufmgr.h" @@ -9569,7 +9568,6 @@ CheckPointGuts(XLogRecPtr checkPointRedo, int flags) { CheckPointRelationMap(); CheckPointReplicationSlots(); - CheckPointSnapBuild(); CheckPointLogicalRewriteHeap(); CheckPointReplicationOrigin(); diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 79bc4a7065..0f4dbdd669 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -40,6 +40,7 @@ #include "pgstat.h" #include "postmaster/custodian.h" #include "postmaster/interrupt.h" +#include "replication/snapbuild.h" #include "storage/bufmgr.h" #include "storage/condition_variable.h" #include "storage/proc.h" @@ -207,6 +208,16 @@ CustodianMain(void) */ RemovePgTempFiles(false, false); + /* + * Remove serialized snapshots that are no longer required by any + * logical replication slot. + * + * It is not important for these to be removed in single-user mode, so + * we don't need any extra handling outside of the custodian process for + * this. + */ + RemoveOldSerializedSnapshots(); + /* Calculate how long to sleep */ end_time = (pg_time_t) time(NULL); elapsed_secs = end_time - start_time; diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 83fca8a77d..466a6478f3 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -125,6 +125,7 @@ #include "access/xact.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/interrupt.h" #include "replication/logical.h" #include "replication/reorderbuffer.h" #include "replication/snapbuild.h" @@ -1912,14 +1913,13 @@ snapshot_not_interesting: /* * Remove all serialized snapshots that are not required anymore because no - * slot can need them. This doesn't actually have to run during a checkpoint, - * but it's a convenient point to schedule this. + * slot can need them. * - * NB: We run this during checkpoints even if logical decoding is disabled so - * we cleanup old slots at some point after it got disabled. + * NB: We run this even if logical decoding is disabled so we cleanup old slots + * at some point after it got disabled. */ void -CheckPointSnapBuild(void) +RemoveOldSerializedSnapshots(void) { XLogRecPtr cutoff; XLogRecPtr redo; @@ -1942,7 +1942,8 @@ CheckPointSnapBuild(void) cutoff = redo; snap_dir = AllocateDir("pg_logical/snapshots"); - while ((snap_de = ReadDir(snap_dir, "pg_logical/snapshots")) != NULL) + while (!ShutdownRequestPending && + (snap_de = ReadDir(snap_dir, "pg_logical/snapshots")) != NULL) { uint32 hi; uint32 lo; diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h index d179251aad..55a2beb434 100644 --- a/src/include/replication/snapbuild.h +++ b/src/include/replication/snapbuild.h @@ -57,7 +57,7 @@ struct ReorderBuffer; struct xl_heap_new_cid; struct xl_running_xacts; -extern void CheckPointSnapBuild(void); +extern void RemoveOldSerializedSnapshots(void); extern SnapBuild *AllocateSnapshotBuilder(struct ReorderBuffer *cache, TransactionId xmin_horizon, XLogRecPtr start_lsn, -- 2.25.1 --BXVAT5kNtrzKuDFl Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0006-Move-removal-of-old-logical-rewrite-mapping-files.patch" ^ permalink raw reply [nested|flat] 20+ messages in thread
* testing framework for MVCC & vacuum (freeze) & heap_page_prune etc. @ 2024-12-10 03:31 Andy Fan <[email protected]> 0 siblings, 1 reply; 20+ messages in thread From: Andy Fan @ 2024-12-10 03:31 UTC (permalink / raw) To: pgsql-hackers Hi, There are so many sistuations in this area, and it is easy to broke something. I want to know if we have some existing testing framework for this area (design, code, licence etc). I'm willing to design one myself but it would be better have a ask first to see if there is some existing excellent project I can start with and contribute to. Thanks! -- Best Regards Andy Fan ^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: testing framework for MVCC & vacuum (freeze) & heap_page_prune etc. @ 2024-12-10 03:42 Andy Fan <[email protected]> parent: Andy Fan <[email protected]> 0 siblings, 0 replies; 20+ messages in thread From: Andy Fan @ 2024-12-10 03:42 UTC (permalink / raw) To: pgsql-hackers Andy Fan <[email protected]> writes: > Hi, > > I'm willing to design one myself > but it would be better have a ask first to see if there is some existing > excellent project I can start with and contribute to. Just to show that I'm not a person who taking things for granted, this is my draft in this topic. I know there are lots of things missed and much more details need to be think more, that's why I was asking if we have existing project. ==== Key Concepts: * MVCC - xmin (insert, update) - xmax (delete, update, select for update, epq query) - CommandId (insert/update/delete -> query) - committs (xmin/xmax) XID may be a state of in-progress, committed/aborted, 2pc prepared. When during scanning tuple within a snapshot, challenge 1: transaction stage changes: (a) new xid. (in-progress) (b) in-progress -> commit. (c) in-progress -> abort (d) new xid -> commit/abort. Challenge 2: The old version deleted by hot update prune or vacuum. hot update prune need hot update query. We may also have sub-transaction, sub-transaction can be tested with "statement_level_txn". We also need to review the above on standby (hot-standby case) todo: xid state crash recovery todo: xid stage on standby (read-only). todo: multiple IDs in each transaction. * vacuum, freeze. Besides the MVCC works correctly, we also need the vacuum & freeze works correctly. vacuum_freeze.sql. -- run 'vacuum (freeze) w;' randomly. * error detection: check_cnt.sql -- Check if the table count is 10000 rows all the time. we inject random sleep time after fetching each tuple to simulate long query, this leave the enough time for others to change their transaction stage or prune the dead tuple etc. check_diff.sql -- Check the result and check if it have 1...10000 IDs, only used for troubleshooting. random sleep time is there already. check_vacuum_freeze.sql: -- a PGSQL function to check the pg_stat_user_tables.n_xxx. pg_class.relfrozenxid/relminmxid, they should be able to advance after some period. If it not, some WARNING should be added. -- Best Regards Andy Fan ^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2024-12-10 03:42 UTC | newest] Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-12-06 06:02 [PATCH v16 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v18 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v6 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v10 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v4 5/8] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v9 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v13 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v15 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v10 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v17 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v14 2/3] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v19 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v20 2/4] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v7 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v12 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v11 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v5 5/8] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2021-12-06 06:02 [PATCH v8 5/6] Move removal of old serialized snapshots to custodian. Nathan Bossart <[email protected]> 2024-12-10 03:31 testing framework for MVCC & vacuum (freeze) & heap_page_prune etc. Andy Fan <[email protected]> 2024-12-10 03:42 ` Re: testing framework for MVCC & vacuum (freeze) & heap_page_prune etc. Andy Fan <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox