public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v11 5/6] Move removal of old serialized snapshots to custodian. 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ messages in thread
* Re: Autogenerate some wait events code and documentation @ 2023-08-29 22:55 Michael Paquier <[email protected]> 0 siblings, 0 replies; 19+ messages in thread From: Michael Paquier @ 2023-08-29 22:55 UTC (permalink / raw) To: Alvaro Herrera <[email protected]>; +Cc: Drouvot, Bertrand <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Aug 29, 2023 at 02:21:48PM +0200, Alvaro Herrera wrote: > Yeah, I have a mild preference for keeping the prefix, but it's mild > because I also imagine that if somebody doesn't see the full symbol name > when grepping they will think to remove the prefix. So only -0.1. So, are you fine with the patch as presented? Or are there other things you'd like to see changed in the format? > I think the DOCONLY stuff should be better documented; they make no > sense without looking at the commit message for fa88928470b5. Good point. However, with 0002 in place these are gone. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2023-08-29 22:55 UTC | newest] Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 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 v6 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 v20 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 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 v14 2/3] 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 v9 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 v10 5/6] 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 v10 5/6] 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]> 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 v4 5/8] 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]> 2023-08-29 22:55 Re: Autogenerate some wait events code and documentation Michael Paquier <[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