public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v11 6/6] Move removal of old logical rewrite mapping files to custodian. 24+ messages / 5 participants [nested] [flat]
* [PATCH v11 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2f08fbe8d3..a01edf8a1f 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -117,6 +117,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -124,6 +125,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1183,7 +1185,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1211,6 +1214,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1243,15 +1251,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1289,3 +1289,61 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --envbJBWh7q8WU6mo-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v15 3/4] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..ff4cd8cef9 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index d0fd955d4b..c4d0a22451 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -382,3 +388,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --HcAYCG3uE/tztfnV Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v15-0004-Do-not-delay-shutdown-due-to-long-running-custod.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v16 3/4] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- contrib/test_decoding/expected/rewrite.out | 21 ++++++ contrib/test_decoding/sql/rewrite.sql | 17 +++++ src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++--- src/backend/postmaster/custodian.c | 43 ++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 6 files changed, 154 insertions(+), 10 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index b30999c436..00b505ef67 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -152,6 +152,27 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc COMMIT (6 rows) +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); + ?column? +---------- + t +(1 row) + SELECT pg_drop_replication_slot('regression_slot'); pg_drop_replication_slot -------------------------- diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index 62dead3a9b..767eccbed4 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -100,6 +100,23 @@ VACUUM FULL pg_proc; VACUUM FULL pg_description; VACUUM FULL pg_shdescription; V INSERT INTO replication_example(somedata, testcolumn1, testcolumn3) VALUES (9, 7, 1); SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); +-- make sure custodian cleans up files +CHECKPOINT; +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); + SELECT pg_drop_replication_slot('regression_slot'); DROP TABLE IF EXISTS replication_example; DROP FUNCTION iamalongfunction(); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..ff4cd8cef9 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index d0fd955d4b..c4d0a22451 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -382,3 +388,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --lrZ03NoBR/3+SXJZ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v16-0004-Do-not-delay-shutdown-due-to-long-running-custod.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v17 3/4] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- contrib/test_decoding/expected/rewrite.out | 19 ++++++ contrib/test_decoding/sql/rewrite.sql | 14 ++++ src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++--- src/backend/postmaster/custodian.c | 43 ++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 6 files changed, 149 insertions(+), 10 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index 8b97f15f6f..214a514a0a 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -183,3 +183,22 @@ SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); t (1 row) +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index d268fa559a..d66f70f837 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -122,3 +122,17 @@ BEGIN END $$; SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..ff4cd8cef9 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index d0fd955d4b..c4d0a22451 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -382,3 +388,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --ew6BAiZeqk4r7MaW Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v17-0004-Do-not-delay-shutdown-due-to-long-running-custod.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 6/8] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/heap/rewriteheap.c | 83 +++++++++++++++++++++++---- src/backend/postmaster/checkpointer.c | 33 +++++++++++ src/backend/postmaster/custodian.c | 10 ++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/bgwriter.h | 3 + 5 files changed, 120 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2a53826736..c5a1103687 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,10 +116,13 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/bgwriter.h" +#include "postmaster/interrupt.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" #include "storage/fd.h" +#include "storage/proc.h" #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" @@ -1182,7 +1185,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1210,6 +1214,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + CheckPointSetLogicalRewriteCutoff(cutoff); + if (ProcGlobal->custodianLatch) + SetLatch(ProcGlobal->custodianLatch); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1240,15 +1249,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1286,3 +1287,65 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't interfere with an + * ongoing call to CheckPointLogicalRewriteHeap() that is flushing mappings to + * disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + bool value_set = false; + + cutoff = CheckPointGetLogicalRewriteCutoff(&value_set); + if (!value_set) + return; + + mappings_dir = AllocateDir("pg_logical/mappings"); + while (!ShutdownRequestPending && + (mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 23f691cd47..fe0934e5a6 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -127,6 +127,9 @@ typedef struct uint32 num_backend_writes; /* counts user backend buffer writes */ uint32 num_backend_fsync; /* counts user backend fsync calls */ + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ + bool logical_rewrite_mappings_cutoff_set; + int num_requests; /* current # of requests */ int max_requests; /* allocated array size */ CheckpointerRequest requests[FLEXIBLE_ARRAY_MEMBER]; @@ -1341,3 +1344,33 @@ FirstCallSinceLastCheckpoint(void) return FirstCall; } + +/* + * Used by CheckPointLogicalRewriteHeap() to tell the custodian which logical + * rewrite mapping files it can remove. + */ +void +CheckPointSetLogicalRewriteCutoff(XLogRecPtr cutoff) +{ + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + CheckpointerShmem->logical_rewrite_mappings_cutoff = cutoff; + CheckpointerShmem->logical_rewrite_mappings_cutoff_set = true; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CheckPointGetLogicalRewriteCutoff(bool *value_set) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + cutoff = CheckpointerShmem->logical_rewrite_mappings_cutoff; + *value_set = CheckpointerShmem->logical_rewrite_mappings_cutoff_set; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); + + return cutoff; +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 0f4dbdd669..9c5479b5cf 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -36,6 +36,7 @@ #include <time.h> +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -218,6 +219,15 @@ CustodianMain(void) */ RemoveOldSerializedSnapshots(); + /* + * Remove logical rewrite mapping files that are no longer needed. + * + * 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. + */ + RemoveOldLogicalRewriteMappings(); + /* Calculate how long to sleep */ end_time = (pg_time_t) time(NULL); elapsed_secs = end_time - start_time; diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index aa5c48f219..f493094557 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" void CheckPointLogicalRewriteHeap(void); +void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h index 2882efd67b..051e6732cb 100644 --- a/src/include/postmaster/bgwriter.h +++ b/src/include/postmaster/bgwriter.h @@ -42,4 +42,7 @@ extern void CheckpointerShmemInit(void); extern bool FirstCallSinceLastCheckpoint(void); +extern void CheckPointSetLogicalRewriteCutoff(XLogRecPtr cutoff); +extern XLogRecPtr CheckPointGetLogicalRewriteCutoff(bool *value_set); + #endif /* _BGWRITER_H */ -- 2.25.1 --BXVAT5kNtrzKuDFl Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0007-Use-syncfs-in-CheckPointLogicalRewriteHeap-for-sh.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v5 6/8] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/heap/rewriteheap.c | 83 +++++++++++++++++++++++---- src/backend/postmaster/checkpointer.c | 33 +++++++++++ src/backend/postmaster/custodian.c | 10 ++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/bgwriter.h | 3 + 5 files changed, 120 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2a53826736..c5a1103687 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,10 +116,13 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/bgwriter.h" +#include "postmaster/interrupt.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" #include "storage/fd.h" +#include "storage/proc.h" #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" @@ -1182,7 +1185,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1210,6 +1214,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + CheckPointSetLogicalRewriteCutoff(cutoff); + if (ProcGlobal->custodianLatch) + SetLatch(ProcGlobal->custodianLatch); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1240,15 +1249,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1286,3 +1287,65 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't interfere with an + * ongoing call to CheckPointLogicalRewriteHeap() that is flushing mappings to + * disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + bool value_set = false; + + cutoff = CheckPointGetLogicalRewriteCutoff(&value_set); + if (!value_set) + return; + + mappings_dir = AllocateDir("pg_logical/mappings"); + while (!ShutdownRequestPending && + (mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 4488e3a443..666f2a0368 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -128,6 +128,9 @@ typedef struct uint32 num_backend_writes; /* counts user backend buffer writes */ uint32 num_backend_fsync; /* counts user backend fsync calls */ + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ + bool logical_rewrite_mappings_cutoff_set; + int num_requests; /* current # of requests */ int max_requests; /* allocated array size */ CheckpointerRequest requests[FLEXIBLE_ARRAY_MEMBER]; @@ -1342,3 +1345,33 @@ FirstCallSinceLastCheckpoint(void) return FirstCall; } + +/* + * Used by CheckPointLogicalRewriteHeap() to tell the custodian which logical + * rewrite mapping files it can remove. + */ +void +CheckPointSetLogicalRewriteCutoff(XLogRecPtr cutoff) +{ + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + CheckpointerShmem->logical_rewrite_mappings_cutoff = cutoff; + CheckpointerShmem->logical_rewrite_mappings_cutoff_set = true; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CheckPointGetLogicalRewriteCutoff(bool *value_set) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + cutoff = CheckpointerShmem->logical_rewrite_mappings_cutoff; + *value_set = CheckpointerShmem->logical_rewrite_mappings_cutoff_set; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); + + return cutoff; +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 8591c5db9b..7f914a617f 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -36,6 +36,7 @@ #include <time.h> +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -219,6 +220,15 @@ CustodianMain(void) */ RemoveOldSerializedSnapshots(); + /* + * Remove logical rewrite mapping files that are no longer needed. + * + * 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. + */ + RemoveOldLogicalRewriteMappings(); + /* Calculate how long to sleep */ end_time = (pg_time_t) time(NULL); elapsed_secs = end_time - start_time; diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index aa5c48f219..f493094557 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" void CheckPointLogicalRewriteHeap(void); +void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h index 2882efd67b..051e6732cb 100644 --- a/src/include/postmaster/bgwriter.h +++ b/src/include/postmaster/bgwriter.h @@ -42,4 +42,7 @@ extern void CheckpointerShmemInit(void); extern bool FirstCallSinceLastCheckpoint(void); +extern void CheckPointSetLogicalRewriteCutoff(XLogRecPtr cutoff); +extern XLogRecPtr CheckPointGetLogicalRewriteCutoff(bool *value_set); + #endif /* _BGWRITER_H */ -- 2.25.1 --2oS5YaxWCcQjTEyO Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0007-Use-syncfs-in-CheckPointLogicalRewriteHeap-for-sh.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v10 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2f08fbe8d3..a01edf8a1f 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -117,6 +117,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -124,6 +125,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1183,7 +1185,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1211,6 +1214,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1243,15 +1251,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1289,3 +1289,61 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 353cbb2924..965372b5ff 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --+QahgC5+KEYLbs62-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v18 3/4] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- contrib/test_decoding/expected/rewrite.out | 19 ++++++ contrib/test_decoding/sql/rewrite.sql | 14 ++++ src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++--- src/backend/postmaster/custodian.c | 43 ++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 6 files changed, 149 insertions(+), 10 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index 8b97f15f6f..214a514a0a 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -183,3 +183,22 @@ SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); t (1 row) +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index d268fa559a..d66f70f837 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -122,3 +122,17 @@ BEGIN END $$; SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..ff4cd8cef9 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 9382d524a6..33185e9913 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -377,3 +383,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --LZvS9be/3tNcYl/X Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v18-0004-Do-not-delay-shutdown-due-to-long-running-custod.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v19 3/4] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- contrib/test_decoding/expected/rewrite.out | 19 ++++++ contrib/test_decoding/sql/rewrite.sql | 14 ++++ src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++--- src/backend/postmaster/custodian.c | 43 ++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 6 files changed, 149 insertions(+), 10 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index 8b97f15f6f..214a514a0a 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -183,3 +183,22 @@ SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); t (1 row) +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index d268fa559a..d66f70f837 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -122,3 +122,17 @@ BEGIN END $$; SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 8993c1ed5a..9ea0f81ac3 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 4e0ce1f7b3..4cbd89fae9 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -377,3 +383,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 1125457053..dc3eb3e308 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --x+6KMIRAuhnl3hBn Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v19-0004-Do-not-delay-shutdown-due-to-long-running-custod.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v20 3/4] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- contrib/test_decoding/expected/rewrite.out | 19 ++++++ contrib/test_decoding/sql/rewrite.sql | 14 ++++ src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++--- src/backend/postmaster/custodian.c | 43 ++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 6 files changed, 149 insertions(+), 10 deletions(-) diff --git a/contrib/test_decoding/expected/rewrite.out b/contrib/test_decoding/expected/rewrite.out index 8b97f15f6f..214a514a0a 100644 --- a/contrib/test_decoding/expected/rewrite.out +++ b/contrib/test_decoding/expected/rewrite.out @@ -183,3 +183,22 @@ SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); t (1 row) +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); + ?column? +---------- + t +(1 row) + diff --git a/contrib/test_decoding/sql/rewrite.sql b/contrib/test_decoding/sql/rewrite.sql index d268fa559a..d66f70f837 100644 --- a/contrib/test_decoding/sql/rewrite.sql +++ b/contrib/test_decoding/sql/rewrite.sql @@ -122,3 +122,17 @@ BEGIN END $$; SELECT count(*) = 0 FROM pg_ls_logicalsnapdir(); +DO $$ +DECLARE + mappings_removed bool; + loops int := 0; +BEGIN + LOOP + mappings_removed := count(*) = 0 FROM pg_ls_logicalmapdir(); + IF mappings_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_logicalmapdir(); diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 8993c1ed5a..9ea0f81ac3 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 4e0ce1f7b3..4cbd89fae9 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -377,3 +383,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 1125457053..dc3eb3e308 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --CE+1k2dSO48ffgeK Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v20-0004-Do-not-delay-shutdown-due-to-long-running-custod.patch" ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v10 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2f08fbe8d3..a01edf8a1f 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -117,6 +117,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -124,6 +125,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1183,7 +1185,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1211,6 +1214,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1243,15 +1251,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1289,3 +1289,61 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 353cbb2924..965372b5ff 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --+QahgC5+KEYLbs62-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v6 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. --- src/backend/access/heap/rewriteheap.c | 79 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 44 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 5 ++ 4 files changed, 119 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2a53826736..edeab65e60 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -1182,7 +1183,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1210,6 +1212,10 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + CustodianSetLogicalRewriteCutoff(cutoff); + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1240,15 +1246,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1286,3 +1284,64 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't interfere with an + * ongoing call to CheckPointLogicalRewriteHeap() that is flushing mappings to + * disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + bool value_set = false; + + cutoff = CustodianGetLogicalRewriteCutoff(&value_set); + if (!value_set) + return; + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 861de882c6..0ce4edcf61 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -27,6 +27,7 @@ #include <time.h> +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -46,6 +47,9 @@ typedef struct { slock_t cust_lck; int cust_flags; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ + bool logical_rewrite_mappings_cutoff_set; } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -222,6 +226,16 @@ CustodianMain(void) if (flags & CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS) RemoveOldSerializedSnapshots(); + /* + * Remove logical rewrite mapping files that are no longer needed. + * + * 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_REWRITE_MAPPINGS) + RemoveOldLogicalRewriteMappings(); + /* Calculate how long to sleep */ end_time = (pg_time_t) time(NULL); elapsed_secs = end_time - start_time; @@ -274,3 +288,33 @@ RequestCustodian(int flags) if (ProcGlobal->custodianLatch) SetLatch(ProcGlobal->custodianLatch); } + +/* + * Used by CheckPointLogicalRewriteHeap() to tell the custodian which logical + * rewrite mapping files it can remove. + */ +void +CustodianSetLogicalRewriteCutoff(XLogRecPtr cutoff) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = cutoff; + CustodianShmem->logical_rewrite_mappings_cutoff_set = true; + SpinLockRelease(&CustodianShmem->cust_lck); +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(bool *value_set) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + *value_set = CustodianShmem->logical_rewrite_mappings_cutoff_set; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 3e27790b3f..61d7aa8ed8 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 769c07f2c9..1af96ebd02 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,13 +12,18 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(int flags); +extern void CustodianSetLogicalRewriteCutoff(XLogRecPtr cutoff); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(bool *value_set); /* flags for RequestCustodian() */ #define CUSTODIAN_REMOVE_TEMP_FILES 0x0001 #define CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS 0x0002 +#define CUSTODIAN_REMOVE_REWRITE_MAPPINGS 0x0004 #endif /* _CUSTODIAN_H */ -- 2.25.1 --liOOAslEiF7prFVr-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v7 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 197f06b5ec..6de1fb19de 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1182,7 +1184,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1210,6 +1213,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1240,15 +1248,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1286,3 +1286,61 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 353cbb2924..965372b5ff 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --/04w6evG8XlLl3ft-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v8 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 9dd885d936..a08dd4a524 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1182,7 +1184,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1210,6 +1213,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1240,15 +1248,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1286,3 +1286,61 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 353cbb2924..965372b5ff 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --X1bOJ3K7DJ5YkBrT-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v9 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 9dd885d936..a08dd4a524 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1182,7 +1184,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1210,6 +1213,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1240,15 +1248,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1286,3 +1286,61 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + struct stat statbuf; + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + if (lstat(path, &statbuf) == 0 && !S_ISREG(statbuf.st_mode)) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 353cbb2924..965372b5ff 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --y0ulUmNC+osPPQO6-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v12 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 80 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 ++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 118 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..07976504cc 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1247,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1285,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --UlVJffcvxoiEqYs2-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v13 6/6] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 80 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 ++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 118 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..07976504cc 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,11 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, + !IsUnderPostmaster, + LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1247,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1285,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index 855a756ca0..d4be19e5de 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(bool retry); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -73,6 +78,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}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -384,3 +390,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index 37334941cc..f177d55159 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -20,6 +22,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_TEMP_FILES, CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -29,5 +32,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, bool immediate, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --CE+1k2dSO48ffgeK-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v14 3/3] Move removal of old logical rewrite mapping files to custodian. @ 2021-12-13 06:07 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Nathan Bossart @ 2021-12-13 06:07 UTC (permalink / raw) If there are many such files to remove, checkpoints can take much longer. To avoid this, move this work to the newly-introduced custodian process. Since the mapping files include 32-bit transaction IDs, there is a risk of wraparound if the files are not cleaned up fast enough. Removing these files in checkpoints offered decent wraparound protection simply due to the relatively high frequency of checkpointing. With this change, servers should still clean up mappings files with decently high frequency, but in theory the wraparound risk might worsen for some (e.g., if the custodian is spending a lot of time on a different task). Given this is an existing problem, this change makes no effort to handle the wraparound risk, and it is left as a future exercise. --- src/backend/access/heap/rewriteheap.c | 78 +++++++++++++++++++++++---- src/backend/postmaster/custodian.c | 43 +++++++++++++++ src/include/access/rewriteheap.h | 1 + src/include/postmaster/custodian.h | 4 ++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index 2fe9e48e50..ff4cd8cef9 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -116,6 +116,7 @@ #include "lib/ilist.h" #include "miscadmin.h" #include "pgstat.h" +#include "postmaster/custodian.h" #include "replication/logical.h" #include "replication/slot.h" #include "storage/bufmgr.h" @@ -123,6 +124,7 @@ #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" #include "utils/rel.h" /* @@ -1179,7 +1181,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r) * Perform a checkpoint for logical rewrite mappings * * This serves two tasks: - * 1) Remove all mappings not needed anymore based on the logical restart LSN + * 1) Alert the custodian to remove all mappings not needed anymore based on the + * logical restart LSN * 2) Flush all remaining mappings to disk, so that replay after a checkpoint * only has to deal with the parts of a mapping that have been written out * after the checkpoint started. @@ -1207,6 +1210,9 @@ CheckPointLogicalRewriteHeap(void) if (cutoff != InvalidXLogRecPtr && redo < cutoff) cutoff = redo; + /* let the custodian know what it can remove */ + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS, LSNGetDatum(cutoff)); + mappings_dir = AllocateDir("pg_logical/mappings"); while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) { @@ -1239,15 +1245,7 @@ CheckPointLogicalRewriteHeap(void) lsn = ((uint64) hi) << 32 | lo; - if (lsn < cutoff || cutoff == InvalidXLogRecPtr) - { - elog(DEBUG1, "removing logical rewrite file \"%s\"", path); - if (unlink(path) < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not remove file \"%s\": %m", path))); - } - else + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) { /* on some operating systems fsyncing a file requires O_RDWR */ int fd = OpenTransientFile(path, O_RDWR | PG_BINARY); @@ -1285,3 +1283,63 @@ CheckPointLogicalRewriteHeap(void) /* persist directory entries to disk */ fsync_fname("pg_logical/mappings", true); } + +/* + * Remove all mappings not needed anymore based on the logical restart LSN saved + * by the checkpointer. We use this saved value instead of calling + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't try to remove + * files that a concurrent call to CheckPointLogicalRewriteHeap() is trying to + * flush to disk. + */ +void +RemoveOldLogicalRewriteMappings(void) +{ + XLogRecPtr cutoff; + DIR *mappings_dir; + struct dirent *mapping_de; + char path[MAXPGPATH + 20]; + + cutoff = CustodianGetLogicalRewriteCutoff(); + + mappings_dir = AllocateDir("pg_logical/mappings"); + while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL) + { + Oid dboid; + Oid relid; + XLogRecPtr lsn; + TransactionId rewrite_xid; + TransactionId create_xid; + uint32 hi, + lo; + PGFileType de_type; + + if (strcmp(mapping_de->d_name, ".") == 0 || + strcmp(mapping_de->d_name, "..") == 0) + continue; + + snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name); + de_type = get_dirent_type(path, mapping_de, false, DEBUG1); + + if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG) + continue; + + /* Skip over files that cannot be ours. */ + if (strncmp(mapping_de->d_name, "map-", 4) != 0) + continue; + + if (sscanf(mapping_de->d_name, LOGICAL_REWRITE_FORMAT, + &dboid, &relid, &hi, &lo, &rewrite_xid, &create_xid) != 6) + elog(ERROR, "could not parse filename \"%s\"", mapping_de->d_name); + + lsn = ((uint64) hi) << 32 | lo; + if (lsn >= cutoff && cutoff != InvalidXLogRecPtr) + continue; + + elog(DEBUG1, "removing logical rewrite file \"%s\"", path); + if (unlink(path) < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not remove file \"%s\": %m", path))); + } + FreeDir(mappings_dir); +} diff --git a/src/backend/postmaster/custodian.c b/src/backend/postmaster/custodian.c index d0fd955d4b..c4d0a22451 100644 --- a/src/backend/postmaster/custodian.c +++ b/src/backend/postmaster/custodian.c @@ -21,6 +21,7 @@ */ #include "postgres.h" +#include "access/rewriteheap.h" #include "libpq/pqsignal.h" #include "pgstat.h" #include "postmaster/custodian.h" @@ -33,11 +34,13 @@ #include "storage/procsignal.h" #include "storage/smgr.h" #include "utils/memutils.h" +#include "utils/pg_lsn.h" static void DoCustodianTasks(void); static CustodianTask CustodianGetNextTask(void); static void CustodianEnqueueTask(CustodianTask task); static const struct cust_task_funcs_entry *LookupCustodianFunctions(CustodianTask task); +static void CustodianSetLogicalRewriteCutoff(Datum arg); typedef struct { @@ -45,6 +48,8 @@ typedef struct CustodianTask task_queue_elems[NUM_CUSTODIAN_TASKS]; int task_queue_head; + + XLogRecPtr logical_rewrite_mappings_cutoff; /* can remove older mappings */ } CustodianShmemStruct; static CustodianShmemStruct *CustodianShmem; @@ -72,6 +77,7 @@ struct cust_task_funcs_entry */ static const struct cust_task_funcs_entry cust_task_functions[] = { {CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, RemoveOldSerializedSnapshots, NULL}, + {CUSTODIAN_REMOVE_REWRITE_MAPPINGS, RemoveOldLogicalRewriteMappings, CustodianSetLogicalRewriteCutoff}, {INVALID_CUSTODIAN_TASK, NULL, NULL} /* must be last */ }; @@ -382,3 +388,40 @@ LookupCustodianFunctions(CustodianTask task) elog(ERROR, "could not lookup functions for custodian task %d", task); pg_unreachable(); } + +/* + * Stores the provided cutoff LSN in the custodian's shared memory. + * + * It's okay if the cutoff LSN is updated before a previously set cutoff has + * been used for cleaning up files. If that happens, it just means that the + * next invocation of RemoveOldLogicalRewriteMappings() will use a more accurate + * cutoff. + */ +static void +CustodianSetLogicalRewriteCutoff(Datum arg) +{ + SpinLockAcquire(&CustodianShmem->cust_lck); + CustodianShmem->logical_rewrite_mappings_cutoff = DatumGetLSN(arg); + SpinLockRelease(&CustodianShmem->cust_lck); + + /* if pass-by-ref, free Datum memory */ +#ifndef USE_FLOAT8_BYVAL + pfree(DatumGetPointer(arg)); +#endif +} + +/* + * Used by the custodian to determine which logical rewrite mapping files it can + * remove. + */ +XLogRecPtr +CustodianGetLogicalRewriteCutoff(void) +{ + XLogRecPtr cutoff; + + SpinLockAcquire(&CustodianShmem->cust_lck); + cutoff = CustodianShmem->logical_rewrite_mappings_cutoff; + SpinLockRelease(&CustodianShmem->cust_lck); + + return cutoff; +} diff --git a/src/include/access/rewriteheap.h b/src/include/access/rewriteheap.h index 5cc04756a5..bc875330d7 100644 --- a/src/include/access/rewriteheap.h +++ b/src/include/access/rewriteheap.h @@ -53,5 +53,6 @@ typedef struct LogicalRewriteMappingData */ #define LOGICAL_REWRITE_FORMAT "map-%x-%x-%X_%X-%x-%x" extern void CheckPointLogicalRewriteHeap(void); +extern void RemoveOldLogicalRewriteMappings(void); #endif /* REWRITE_HEAP_H */ diff --git a/src/include/postmaster/custodian.h b/src/include/postmaster/custodian.h index ab6d4283b9..00280c203b 100644 --- a/src/include/postmaster/custodian.h +++ b/src/include/postmaster/custodian.h @@ -12,6 +12,8 @@ #ifndef _CUSTODIAN_H #define _CUSTODIAN_H +#include "access/xlogdefs.h" + /* * If you add a new task here, be sure to add its corresponding function * pointers to cust_task_functions in custodian.c. @@ -19,6 +21,7 @@ typedef enum CustodianTask { CUSTODIAN_REMOVE_SERIALIZED_SNAPSHOTS, + CUSTODIAN_REMOVE_REWRITE_MAPPINGS, NUM_CUSTODIAN_TASKS, /* new tasks go above */ INVALID_CUSTODIAN_TASK @@ -28,5 +31,6 @@ extern void CustodianMain(void) pg_attribute_noreturn(); extern Size CustodianShmemSize(void); extern void CustodianShmemInit(void); extern void RequestCustodian(CustodianTask task, Datum arg); +extern XLogRecPtr CustodianGetLogicalRewriteCutoff(void); #endif /* _CUSTODIAN_H */ -- 2.25.1 --BXVAT5kNtrzKuDFl-- ^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Add a new pg_walinspect function to extract FPIs from WAL records @ 2023-01-05 13:21 Bharath Rupireddy <[email protected]> 0 siblings, 2 replies; 24+ messages in thread From: Bharath Rupireddy @ 2023-01-05 13:21 UTC (permalink / raw) To: Drouvot, Bertrand <[email protected]>; PostgreSQL Hackers <[email protected]> On Wed, Jan 4, 2023 at 8:19 PM Drouvot, Bertrand <[email protected]> wrote: > > I think it makes sense to somehow align the pg_walinspect functions with the pg_waldump "features". > And since [1] added FPI "extraction" then +1 for the proposed patch in this thread. > > > [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d497093cbecccf6df26365e06a5f8f861... > > [2] https://postgr.es/m/CAOxo6XKjQb2bMSBRpePf3ZpzfNTwjQUc4Tafh21=jzjX6bX8CA@mail.gmail.com > > I just have a few comments: Thanks for reviewing. > + > +/* > + * Get full page images and their info associated with a given WAL record. > + */ > > > + <para> > + Gets raw full page images and their information associated with all the > + valid WAL records between <replaceable>start_lsn</replaceable> and > + <replaceable>end_lsn</replaceable>. Returns one row per full page image. > > Worth to add a few words about decompression too? Done. > What about adding a few words about compression? (like "Decompression is applied if necessary"?) > > > + /* Full page exists, so let's output it. */ > + if (!RestoreBlockImage(record, block_id, page)) > > "Full page exists, so let's output its info and content." instead? Done. > I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that both > extracted images are the same and matches the one modified right after the checkpoint. > > What do you think? (could be done later in another patch though). I think pageinspect can be used here. We can fetch the raw page from the table after the checkpoint and raw FPI from the WAL record logged as part of the update. I've tried to do so [1], but I see a slight difference in the raw output. The expectation is that they both be the same. It might be that the update operation logs the FPI with some more info set (prune_xid). I'll try to see why it is so. I'm attaching the v2 patch for further review. [1] SELECT * FROM page_header(:'page_from_table'); lsn | checksum | flags | lower | upper | special | pagesize | version | prune_xid -----------+----------+-------+-------+-------+---------+----------+---------+----------- 0/1891D78 | 0 | 0 | 40 | 8064 | 8192 | 8192 | 4 | 0 (1 row) SELECT * FROM page_header(:'page_from_wal'); lsn | checksum | flags | lower | upper | special | pagesize | version | prune_xid -----------+----------+-------+-------+-------+---------+----------+---------+----------- 0/1891D78 | 0 | 0 | 44 | 8032 | 8192 | 8192 | 4 | 735 (1 row) -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com Attachments: [application/octet-stream] v2-0001-Add-FPI-extract-function-to-pg_walinspect.patch (15.5K, ../../CALj2ACX=QtDfj_7DzzRbV+s+oCG_YafY6PCp+cJ+4mZbrn-cnQ@mail.gmail.com/2-v2-0001-Add-FPI-extract-function-to-pg_walinspect.patch) download | inline diff: From 53aa9e78b80f3fa09ba299c2a165eeb4e87e946f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Thu, 5 Jan 2023 11:15:45 +0000 Subject: [PATCH v2] Add FPI extract function to pg_walinspect --- contrib/pg_walinspect/Makefile | 2 +- .../pg_walinspect/expected/pg_walinspect.out | 46 ++++++++ contrib/pg_walinspect/meson.build | 1 + contrib/pg_walinspect/pg_walinspect.c | 110 ++++++++++++++++++ contrib/pg_walinspect/pg_walinspect.control | 2 +- contrib/pg_walinspect/sql/pg_walinspect.sql | 32 +++++ doc/src/sgml/pgwalinspect.sgml | 49 ++++++++ 7 files changed, 240 insertions(+), 2 deletions(-) diff --git a/contrib/pg_walinspect/Makefile b/contrib/pg_walinspect/Makefile index 960530eb6c..7033878a79 100644 --- a/contrib/pg_walinspect/Makefile +++ b/contrib/pg_walinspect/Makefile @@ -7,7 +7,7 @@ OBJS = \ PGFILEDESC = "pg_walinspect - functions to inspect contents of PostgreSQL Write-Ahead Log" EXTENSION = pg_walinspect -DATA = pg_walinspect--1.0.sql +DATA = pg_walinspect--1.0.sql pg_walinspect--1.0--1.1.sql REGRESS = pg_walinspect diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out index a1ee743457..5cd28402ad 100644 --- a/contrib/pg_walinspect/expected/pg_walinspect.out +++ b/contrib/pg_walinspect/expected/pg_walinspect.out @@ -73,6 +73,27 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2' t (1 row) +-- =================================================================== +-- Tests for getting full page images +-- =================================================================== +SELECT pg_current_wal_lsn() AS wal_lsn3 \gset +-- Force FPWs on the next writes. +CHECKPOINT; +UPDATE sample_tbl SET col1 = col1 + 1; +SELECT 1 FROM pg_switch_wal(); + ?column? +---------- + 1 +(1 row) + +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset +SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid'; + ok +---- + t +(1 row) + -- =================================================================== -- Tests for permissions -- =================================================================== @@ -98,6 +119,13 @@ SELECT has_function_privilege('regress_pg_walinspect', f (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no + has_function_privilege +------------------------ + f +(1 row) + -- Functions accessible by users with role pg_read_server_files GRANT pg_read_server_files TO regress_pg_walinspect; SELECT has_function_privilege('regress_pg_walinspect', @@ -121,6 +149,13 @@ SELECT has_function_privilege('regress_pg_walinspect', t (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + REVOKE pg_read_server_files FROM regress_pg_walinspect; -- Superuser can grant execute to other users GRANT EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) @@ -129,6 +164,8 @@ GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) TO regress_pg_walinspect; GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO regress_pg_walinspect; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + TO regress_pg_walinspect; SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes has_function_privilege @@ -150,12 +187,21 @@ SELECT has_function_privilege('regress_pg_walinspect', t (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) FROM regress_pg_walinspect; REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) FROM regress_pg_walinspect; REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM regress_pg_walinspect; +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + FROM regress_pg_walinspect; -- =================================================================== -- Clean up -- =================================================================== diff --git a/contrib/pg_walinspect/meson.build b/contrib/pg_walinspect/meson.build index f6e912a7d8..bf7b79b1b7 100644 --- a/contrib/pg_walinspect/meson.build +++ b/contrib/pg_walinspect/meson.build @@ -19,6 +19,7 @@ contrib_targets += pg_walinspect install_data( 'pg_walinspect.control', 'pg_walinspect--1.0.sql', + 'pg_walinspect--1.0--1.1.sql', kwargs: contrib_data_args, ) diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c index 08a4c2fb52..8f992960e8 100644 --- a/contrib/pg_walinspect/pg_walinspect.c +++ b/contrib/pg_walinspect/pg_walinspect.c @@ -35,6 +35,7 @@ PG_FUNCTION_INFO_V1(pg_get_wal_records_info); PG_FUNCTION_INFO_V1(pg_get_wal_records_info_till_end_of_wal); PG_FUNCTION_INFO_V1(pg_get_wal_stats); PG_FUNCTION_INFO_V1(pg_get_wal_stats_till_end_of_wal); +PG_FUNCTION_INFO_V1(pg_get_wal_fpi_info); static bool IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn); static XLogReaderState *InitXLogReaderState(XLogRecPtr lsn); @@ -55,6 +56,7 @@ static void FillXLogStatsRow(const char *name, uint64 n, uint64 total_count, Datum *values, bool *nulls, uint32 ncols); static void GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn, bool stats_per_record); +static void GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record); /* * Check if the given LSN is in future. Also, return the LSN up to which the @@ -616,3 +618,111 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +/* + * Get full page images and their info associated with a given WAL record. + * Decompression is applied to the blocks, if necessary. + */ +static void +GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record) +{ +#define PG_GET_WAL_FPI_INFO_COLS 7 + int block_id; + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) + { + PGAlignedBlock buf; + Page page; + bytea *raw_page; + BlockNumber blk; + RelFileLocator rnode; + ForkNumber fork; + Datum values[PG_GET_WAL_FPI_INFO_COLS] = {0}; + bool nulls[PG_GET_WAL_FPI_INFO_COLS] = {0}; + int i = 0; + + if (!XLogRecHasBlockRef(record, block_id)) + continue; + + if (!XLogRecHasBlockImage(record, block_id)) + continue; + + page = (Page) buf.data; + + if (!RestoreBlockImage(record, block_id, page)) + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg_internal("%s", record->errormsg_buf))); + + /* Full page exists, so let's output its info and content. */ + (void) XLogRecGetBlockTagExtended(record, block_id, + &rnode, &fork, &blk, NULL); + + values[i++] = LSNGetDatum(record->ReadRecPtr); + values[i++] = ObjectIdGetDatum(rnode.spcOid); + values[i++] = ObjectIdGetDatum(rnode.dbOid); + values[i++] = ObjectIdGetDatum(rnode.relNumber); + values[i++] = Int64GetDatum((int64) blk); + + if (fork >= 0 && fork <= MAX_FORKNUM) + values[i++] = CStringGetTextDatum(forkNames[fork]); + else + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg_internal("invalid fork number: %u", fork))); + + /* Initialize bytea buffer to copy the FPI to. */ + raw_page = (bytea *) palloc(BLCKSZ + VARHDRSZ); + SET_VARSIZE(raw_page, BLCKSZ + VARHDRSZ); + + /* Take a verbatim copy of the FPI. */ + memcpy(VARDATA(raw_page), page, BLCKSZ); + + values[i++] = PointerGetDatum(raw_page); + + Assert(i == PG_GET_WAL_FPI_INFO_COLS); + + tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, + values, nulls); + } + +#undef PG_GET_WAL_FPI_INFO_COLS +} + +/* + * Get full page images and their info of all WAL records between start LSN and + * end LSN. Decompression is applied to the blocks, if necessary. + * + * This function emits an error if a future start or end WAL LSN i.e. WAL LSN + * the database system doesn't know about is specified. + */ +Datum +pg_get_wal_fpi_info(PG_FUNCTION_ARGS) +{ + XLogRecPtr start_lsn; + XLogRecPtr end_lsn; + XLogReaderState *xlogreader; + + start_lsn = PG_GETARG_LSN(0); + end_lsn = PG_GETARG_LSN(1); + + end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn); + + InitMaterializedSRF(fcinfo, 0); + + xlogreader = InitXLogReaderState(start_lsn); + + while (ReadNextXLogRecord(xlogreader) && + xlogreader->EndRecPtr <= end_lsn) + { + GetWALFPIInfo(fcinfo, xlogreader); + + CHECK_FOR_INTERRUPTS(); + } + + pfree(xlogreader->private_data); + XLogReaderFree(xlogreader); + + PG_RETURN_VOID(); +} diff --git a/contrib/pg_walinspect/pg_walinspect.control b/contrib/pg_walinspect/pg_walinspect.control index 017e56a2bb..efa3cb2cfe 100644 --- a/contrib/pg_walinspect/pg_walinspect.control +++ b/contrib/pg_walinspect/pg_walinspect.control @@ -1,5 +1,5 @@ # pg_walinspect extension comment = 'functions to inspect contents of PostgreSQL Write-Ahead Log' -default_version = '1.0' +default_version = '1.1' module_pathname = '$libdir/pg_walinspect' relocatable = true diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql index 1b265ea7bc..25f707de44 100644 --- a/contrib/pg_walinspect/sql/pg_walinspect.sql +++ b/contrib/pg_walinspect/sql/pg_walinspect.sql @@ -52,6 +52,23 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2' SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2') WHERE resource_manager = 'Heap' AND record_type = 'INSERT'; +-- =================================================================== +-- Tests for getting full page images +-- =================================================================== +SELECT pg_current_wal_lsn() AS wal_lsn3 \gset + +-- Force FPWs on the next writes. +CHECKPOINT; + +UPDATE sample_tbl SET col1 = col1 + 1; + +SELECT 1 FROM pg_switch_wal(); + +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset + +SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid'; + -- =================================================================== -- Tests for permissions -- =================================================================== @@ -66,6 +83,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- no +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no + -- Functions accessible by users with role pg_read_server_files GRANT pg_read_server_files TO regress_pg_walinspect; @@ -79,6 +99,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + REVOKE pg_read_server_files FROM regress_pg_walinspect; -- Superuser can grant execute to other users @@ -91,6 +114,9 @@ GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO regress_pg_walinspect; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + TO regress_pg_walinspect; + SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes @@ -100,6 +126,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) FROM regress_pg_walinspect; @@ -109,6 +138,9 @@ REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM regress_pg_walinspect; +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + FROM regress_pg_walinspect; + -- =================================================================== -- Clean up -- =================================================================== diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 1a1bee7d6a..ef5e5852a9 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -261,6 +261,55 @@ postgres=# select * from pg_get_wal_stats('0/14AFC30', '0/15011D7', true) where </listitem> </varlistentry> + <varlistentry> + <term> + <function> + pg_get_wal_fpi_info(start_lsn pg_lsn, + end_lsn pg_lsn, + lsn OUT pg_lsn, + tablespace_oid OUT oid, + database_oid OUT oid, + relfile_number OUT oid, + block_number OUT int8, + fork_name OUT text, + fpi OUT bytea) + returns setof record + </function> + </term> + + <listitem> + <para> + Gets raw full page images and their information associated with all the + valid WAL records between <replaceable>start_lsn</replaceable> and + <replaceable>end_lsn</replaceable>. Returns one row per full page image. + Decompression is applied to the blocks, if necessary. + If <replaceable>start_lsn</replaceable> or + <replaceable>end_lsn</replaceable> are not yet available, the function + will raise an error. For example, usage of the function is as follows: +<screen> +postgres=# SELECT lsn, tablespace_oid, database_oid, relfile_number, block_number, fork_name, length(fpi) > 0 as fpi_ok FROM pg_get_wal_fpi_info('0/7418E60', '0/7518218'); + lsn | tablespace_oid | database_oid | relfile_number | block_number | fork_name | fpi_ok +-----------+----------------+--------------+----------------+--------------+-----------+-------- + 0/7418F48 | 1663 | 5 | 16419 | 0 | main | t + 0/7419080 | 1663 | 5 | 16422 | 19 | main | t + 0/741B0B8 | 1663 | 5 | 16422 | 22 | main | t + 0/741BF48 | 1663 | 5 | 16422 | 20 | main | t + 0/74257B8 | 1663 | 5 | 16422 | 21 | main | t + 0/742FF30 | 1663 | 5 | 16425 | 42 | main | t + 0/7431F68 | 1663 | 5 | 16425 | 44 | main | t + 0/7439B78 | 1663 | 5 | 16425 | 43 | main | t + 0/74451E8 | 1663 | 5 | 16428 | 199 | main | t + 0/7447220 | 1663 | 5 | 16428 | 221 | main | t + 0/744E458 | 1663 | 5 | 16428 | 200 | main | t + 0/7457CB0 | 1663 | 5 | 16428 | 201 | main | t + 0/7461520 | 1663 | 5 | 16428 | 202 | main | t + 0/746AD90 | 1663 | 5 | 16428 | 203 | main | t +(14 rows) +</screen> + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> -- 2.34.1 ^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Add a new pg_walinspect function to extract FPIs from WAL records @ 2023-01-06 05:54 vignesh C <[email protected]> parent: Bharath Rupireddy <[email protected]> 1 sibling, 0 replies; 24+ messages in thread From: vignesh C @ 2023-01-06 05:54 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; +Cc: Drouvot, Bertrand <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, 5 Jan 2023 at 18:52, Bharath Rupireddy <[email protected]> wrote: > > On Wed, Jan 4, 2023 at 8:19 PM Drouvot, Bertrand > <[email protected]> wrote: > > > > I think it makes sense to somehow align the pg_walinspect functions with the pg_waldump "features". > > And since [1] added FPI "extraction" then +1 for the proposed patch in this thread. > > > > > [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d497093cbecccf6df26365e06a5f8f861... > > > [2] https://postgr.es/m/CAOxo6XKjQb2bMSBRpePf3ZpzfNTwjQUc4Tafh21=jzjX6bX8CA@mail.gmail.com > > > > I just have a few comments: > > Thanks for reviewing. > > > + > > +/* > > + * Get full page images and their info associated with a given WAL record. > > + */ > > > > > > + <para> > > + Gets raw full page images and their information associated with all the > > + valid WAL records between <replaceable>start_lsn</replaceable> and > > + <replaceable>end_lsn</replaceable>. Returns one row per full page image. > > > > Worth to add a few words about decompression too? > > Done. > > > What about adding a few words about compression? (like "Decompression is applied if necessary"?) > > > > > > + /* Full page exists, so let's output it. */ > > + if (!RestoreBlockImage(record, block_id, page)) > > > > "Full page exists, so let's output its info and content." instead? > > Done. > > > I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that both > > extracted images are the same and matches the one modified right after the checkpoint. > > > > What do you think? (could be done later in another patch though). > > I think pageinspect can be used here. We can fetch the raw page from > the table after the checkpoint and raw FPI from the WAL record logged > as part of the update. I've tried to do so [1], but I see a slight > difference in the raw output. The expectation is that they both be the > same. It might be that the update operation logs the FPI with some > more info set (prune_xid). I'll try to see why it is so. > > I'm attaching the v2 patch for further review. I felt one of the files was missing in the patch: [13:39:03.534] contrib/pg_walinspect/meson.build:19:0: ERROR: File pg_walinspect--1.0--1.1.sql does not exist. Please post an updated version for the same. Regards, Vignesh ^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Add a new pg_walinspect function to extract FPIs from WAL records @ 2023-01-06 06:17 Bharath Rupireddy <[email protected]> parent: Bharath Rupireddy <[email protected]> 1 sibling, 1 reply; 24+ messages in thread From: Bharath Rupireddy @ 2023-01-06 06:17 UTC (permalink / raw) To: Drouvot, Bertrand <[email protected]>; PostgreSQL Hackers <[email protected]> On Thu, Jan 5, 2023 at 6:51 PM Bharath Rupireddy <[email protected]> wrote: > > > I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that both > > extracted images are the same and matches the one modified right after the checkpoint. > > > > What do you think? (could be done later in another patch though). > > I think pageinspect can be used here. We can fetch the raw page from > the table after the checkpoint and raw FPI from the WAL record logged > as part of the update. I've tried to do so [1], but I see a slight > difference in the raw output. The expectation is that they both be the > same. It might be that the update operation logs the FPI with some > more info set (prune_xid). I'll try to see why it is so. > > I'm attaching the v2 patch for further review. > > [1] > SELECT * FROM page_header(:'page_from_table'); > lsn | checksum | flags | lower | upper | special | pagesize | > version | prune_xid > -----------+----------+-------+-------+-------+---------+----------+---------+----------- > 0/1891D78 | 0 | 0 | 40 | 8064 | 8192 | 8192 | > 4 | 0 > (1 row) > > SELECT * FROM page_header(:'page_from_wal'); > lsn | checksum | flags | lower | upper | special | pagesize | > version | prune_xid > -----------+----------+-------+-------+-------+---------+----------+---------+----------- > 0/1891D78 | 0 | 0 | 44 | 8032 | 8192 | 8192 | > 4 | 735 > (1 row) Ugh, v2 patch missed the new file added, I'm attaching v3 patch for further review. Sorry for the noise. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com Attachments: [application/octet-stream] v3-0001-Add-FPI-extract-function-to-pg_walinspect.patch (16.6K, ../../CALj2ACXpw+8X2gqzkdVdVm68rsvfAbK=h50PLX7NuV8C1Qcxmg@mail.gmail.com/2-v3-0001-Add-FPI-extract-function-to-pg_walinspect.patch) download | inline diff: From 0618bf43054322927226a1cde5ca5dad5bcbeaa6 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Fri, 6 Jan 2023 06:13:26 +0000 Subject: [PATCH v3] Add FPI extract function to pg_walinspect --- contrib/pg_walinspect/Makefile | 2 +- .../pg_walinspect/expected/pg_walinspect.out | 46 ++++++++ contrib/pg_walinspect/meson.build | 1 + .../pg_walinspect/pg_walinspect--1.0--1.1.sql | 24 ++++ contrib/pg_walinspect/pg_walinspect.c | 110 ++++++++++++++++++ contrib/pg_walinspect/pg_walinspect.control | 2 +- contrib/pg_walinspect/sql/pg_walinspect.sql | 32 +++++ doc/src/sgml/pgwalinspect.sgml | 49 ++++++++ 8 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql diff --git a/contrib/pg_walinspect/Makefile b/contrib/pg_walinspect/Makefile index 960530eb6c..7033878a79 100644 --- a/contrib/pg_walinspect/Makefile +++ b/contrib/pg_walinspect/Makefile @@ -7,7 +7,7 @@ OBJS = \ PGFILEDESC = "pg_walinspect - functions to inspect contents of PostgreSQL Write-Ahead Log" EXTENSION = pg_walinspect -DATA = pg_walinspect--1.0.sql +DATA = pg_walinspect--1.0.sql pg_walinspect--1.0--1.1.sql REGRESS = pg_walinspect diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out index a1ee743457..5cd28402ad 100644 --- a/contrib/pg_walinspect/expected/pg_walinspect.out +++ b/contrib/pg_walinspect/expected/pg_walinspect.out @@ -73,6 +73,27 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2' t (1 row) +-- =================================================================== +-- Tests for getting full page images +-- =================================================================== +SELECT pg_current_wal_lsn() AS wal_lsn3 \gset +-- Force FPWs on the next writes. +CHECKPOINT; +UPDATE sample_tbl SET col1 = col1 + 1; +SELECT 1 FROM pg_switch_wal(); + ?column? +---------- + 1 +(1 row) + +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset +SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid'; + ok +---- + t +(1 row) + -- =================================================================== -- Tests for permissions -- =================================================================== @@ -98,6 +119,13 @@ SELECT has_function_privilege('regress_pg_walinspect', f (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no + has_function_privilege +------------------------ + f +(1 row) + -- Functions accessible by users with role pg_read_server_files GRANT pg_read_server_files TO regress_pg_walinspect; SELECT has_function_privilege('regress_pg_walinspect', @@ -121,6 +149,13 @@ SELECT has_function_privilege('regress_pg_walinspect', t (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + REVOKE pg_read_server_files FROM regress_pg_walinspect; -- Superuser can grant execute to other users GRANT EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) @@ -129,6 +164,8 @@ GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) TO regress_pg_walinspect; GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO regress_pg_walinspect; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + TO regress_pg_walinspect; SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes has_function_privilege @@ -150,12 +187,21 @@ SELECT has_function_privilege('regress_pg_walinspect', t (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) FROM regress_pg_walinspect; REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) FROM regress_pg_walinspect; REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM regress_pg_walinspect; +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + FROM regress_pg_walinspect; -- =================================================================== -- Clean up -- =================================================================== diff --git a/contrib/pg_walinspect/meson.build b/contrib/pg_walinspect/meson.build index f6e912a7d8..bf7b79b1b7 100644 --- a/contrib/pg_walinspect/meson.build +++ b/contrib/pg_walinspect/meson.build @@ -19,6 +19,7 @@ contrib_targets += pg_walinspect install_data( 'pg_walinspect.control', 'pg_walinspect--1.0.sql', + 'pg_walinspect--1.0--1.1.sql', kwargs: contrib_data_args, ) diff --git a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql new file mode 100644 index 0000000000..4ffbd91728 --- /dev/null +++ b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql @@ -0,0 +1,24 @@ +/* contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION pg_walinspect UPDATE TO '1.1'" to load this file. \quit + +-- +-- pg_get_wal_fpi_info() +-- +CREATE FUNCTION pg_get_wal_fpi_info(IN start_lsn pg_lsn, + IN end_lsn pg_lsn, + OUT lsn pg_lsn, + OUT tablespace_oid oid, + OUT database_oid oid, + OUT relfile_number oid, + OUT block_number int8, + OUT fork_name text, + OUT fpi bytea +) +RETURNS SETOF record +AS 'MODULE_PATHNAME', 'pg_get_wal_fpi_info' +LANGUAGE C STRICT PARALLEL SAFE; + +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) TO pg_read_server_files; diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c index 08a4c2fb52..8f992960e8 100644 --- a/contrib/pg_walinspect/pg_walinspect.c +++ b/contrib/pg_walinspect/pg_walinspect.c @@ -35,6 +35,7 @@ PG_FUNCTION_INFO_V1(pg_get_wal_records_info); PG_FUNCTION_INFO_V1(pg_get_wal_records_info_till_end_of_wal); PG_FUNCTION_INFO_V1(pg_get_wal_stats); PG_FUNCTION_INFO_V1(pg_get_wal_stats_till_end_of_wal); +PG_FUNCTION_INFO_V1(pg_get_wal_fpi_info); static bool IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn); static XLogReaderState *InitXLogReaderState(XLogRecPtr lsn); @@ -55,6 +56,7 @@ static void FillXLogStatsRow(const char *name, uint64 n, uint64 total_count, Datum *values, bool *nulls, uint32 ncols); static void GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn, bool stats_per_record); +static void GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record); /* * Check if the given LSN is in future. Also, return the LSN up to which the @@ -616,3 +618,111 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +/* + * Get full page images and their info associated with a given WAL record. + * Decompression is applied to the blocks, if necessary. + */ +static void +GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record) +{ +#define PG_GET_WAL_FPI_INFO_COLS 7 + int block_id; + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) + { + PGAlignedBlock buf; + Page page; + bytea *raw_page; + BlockNumber blk; + RelFileLocator rnode; + ForkNumber fork; + Datum values[PG_GET_WAL_FPI_INFO_COLS] = {0}; + bool nulls[PG_GET_WAL_FPI_INFO_COLS] = {0}; + int i = 0; + + if (!XLogRecHasBlockRef(record, block_id)) + continue; + + if (!XLogRecHasBlockImage(record, block_id)) + continue; + + page = (Page) buf.data; + + if (!RestoreBlockImage(record, block_id, page)) + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg_internal("%s", record->errormsg_buf))); + + /* Full page exists, so let's output its info and content. */ + (void) XLogRecGetBlockTagExtended(record, block_id, + &rnode, &fork, &blk, NULL); + + values[i++] = LSNGetDatum(record->ReadRecPtr); + values[i++] = ObjectIdGetDatum(rnode.spcOid); + values[i++] = ObjectIdGetDatum(rnode.dbOid); + values[i++] = ObjectIdGetDatum(rnode.relNumber); + values[i++] = Int64GetDatum((int64) blk); + + if (fork >= 0 && fork <= MAX_FORKNUM) + values[i++] = CStringGetTextDatum(forkNames[fork]); + else + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg_internal("invalid fork number: %u", fork))); + + /* Initialize bytea buffer to copy the FPI to. */ + raw_page = (bytea *) palloc(BLCKSZ + VARHDRSZ); + SET_VARSIZE(raw_page, BLCKSZ + VARHDRSZ); + + /* Take a verbatim copy of the FPI. */ + memcpy(VARDATA(raw_page), page, BLCKSZ); + + values[i++] = PointerGetDatum(raw_page); + + Assert(i == PG_GET_WAL_FPI_INFO_COLS); + + tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, + values, nulls); + } + +#undef PG_GET_WAL_FPI_INFO_COLS +} + +/* + * Get full page images and their info of all WAL records between start LSN and + * end LSN. Decompression is applied to the blocks, if necessary. + * + * This function emits an error if a future start or end WAL LSN i.e. WAL LSN + * the database system doesn't know about is specified. + */ +Datum +pg_get_wal_fpi_info(PG_FUNCTION_ARGS) +{ + XLogRecPtr start_lsn; + XLogRecPtr end_lsn; + XLogReaderState *xlogreader; + + start_lsn = PG_GETARG_LSN(0); + end_lsn = PG_GETARG_LSN(1); + + end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn); + + InitMaterializedSRF(fcinfo, 0); + + xlogreader = InitXLogReaderState(start_lsn); + + while (ReadNextXLogRecord(xlogreader) && + xlogreader->EndRecPtr <= end_lsn) + { + GetWALFPIInfo(fcinfo, xlogreader); + + CHECK_FOR_INTERRUPTS(); + } + + pfree(xlogreader->private_data); + XLogReaderFree(xlogreader); + + PG_RETURN_VOID(); +} diff --git a/contrib/pg_walinspect/pg_walinspect.control b/contrib/pg_walinspect/pg_walinspect.control index 017e56a2bb..efa3cb2cfe 100644 --- a/contrib/pg_walinspect/pg_walinspect.control +++ b/contrib/pg_walinspect/pg_walinspect.control @@ -1,5 +1,5 @@ # pg_walinspect extension comment = 'functions to inspect contents of PostgreSQL Write-Ahead Log' -default_version = '1.0' +default_version = '1.1' module_pathname = '$libdir/pg_walinspect' relocatable = true diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql index 1b265ea7bc..25f707de44 100644 --- a/contrib/pg_walinspect/sql/pg_walinspect.sql +++ b/contrib/pg_walinspect/sql/pg_walinspect.sql @@ -52,6 +52,23 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2' SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2') WHERE resource_manager = 'Heap' AND record_type = 'INSERT'; +-- =================================================================== +-- Tests for getting full page images +-- =================================================================== +SELECT pg_current_wal_lsn() AS wal_lsn3 \gset + +-- Force FPWs on the next writes. +CHECKPOINT; + +UPDATE sample_tbl SET col1 = col1 + 1; + +SELECT 1 FROM pg_switch_wal(); + +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset + +SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid'; + -- =================================================================== -- Tests for permissions -- =================================================================== @@ -66,6 +83,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- no +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no + -- Functions accessible by users with role pg_read_server_files GRANT pg_read_server_files TO regress_pg_walinspect; @@ -79,6 +99,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + REVOKE pg_read_server_files FROM regress_pg_walinspect; -- Superuser can grant execute to other users @@ -91,6 +114,9 @@ GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO regress_pg_walinspect; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + TO regress_pg_walinspect; + SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes @@ -100,6 +126,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) FROM regress_pg_walinspect; @@ -109,6 +138,9 @@ REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM regress_pg_walinspect; +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + FROM regress_pg_walinspect; + -- =================================================================== -- Clean up -- =================================================================== diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 1a1bee7d6a..ef5e5852a9 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -261,6 +261,55 @@ postgres=# select * from pg_get_wal_stats('0/14AFC30', '0/15011D7', true) where </listitem> </varlistentry> + <varlistentry> + <term> + <function> + pg_get_wal_fpi_info(start_lsn pg_lsn, + end_lsn pg_lsn, + lsn OUT pg_lsn, + tablespace_oid OUT oid, + database_oid OUT oid, + relfile_number OUT oid, + block_number OUT int8, + fork_name OUT text, + fpi OUT bytea) + returns setof record + </function> + </term> + + <listitem> + <para> + Gets raw full page images and their information associated with all the + valid WAL records between <replaceable>start_lsn</replaceable> and + <replaceable>end_lsn</replaceable>. Returns one row per full page image. + Decompression is applied to the blocks, if necessary. + If <replaceable>start_lsn</replaceable> or + <replaceable>end_lsn</replaceable> are not yet available, the function + will raise an error. For example, usage of the function is as follows: +<screen> +postgres=# SELECT lsn, tablespace_oid, database_oid, relfile_number, block_number, fork_name, length(fpi) > 0 as fpi_ok FROM pg_get_wal_fpi_info('0/7418E60', '0/7518218'); + lsn | tablespace_oid | database_oid | relfile_number | block_number | fork_name | fpi_ok +-----------+----------------+--------------+----------------+--------------+-----------+-------- + 0/7418F48 | 1663 | 5 | 16419 | 0 | main | t + 0/7419080 | 1663 | 5 | 16422 | 19 | main | t + 0/741B0B8 | 1663 | 5 | 16422 | 22 | main | t + 0/741BF48 | 1663 | 5 | 16422 | 20 | main | t + 0/74257B8 | 1663 | 5 | 16422 | 21 | main | t + 0/742FF30 | 1663 | 5 | 16425 | 42 | main | t + 0/7431F68 | 1663 | 5 | 16425 | 44 | main | t + 0/7439B78 | 1663 | 5 | 16425 | 43 | main | t + 0/74451E8 | 1663 | 5 | 16428 | 199 | main | t + 0/7447220 | 1663 | 5 | 16428 | 221 | main | t + 0/744E458 | 1663 | 5 | 16428 | 200 | main | t + 0/7457CB0 | 1663 | 5 | 16428 | 201 | main | t + 0/7461520 | 1663 | 5 | 16428 | 202 | main | t + 0/746AD90 | 1663 | 5 | 16428 | 203 | main | t +(14 rows) +</screen> + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> -- 2.34.1 ^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Add a new pg_walinspect function to extract FPIs from WAL records @ 2023-01-06 17:41 Bharath Rupireddy <[email protected]> parent: Bharath Rupireddy <[email protected]> 0 siblings, 1 reply; 24+ messages in thread From: Bharath Rupireddy @ 2023-01-06 17:41 UTC (permalink / raw) To: Drouvot, Bertrand <[email protected]>; PostgreSQL Hackers <[email protected]> On Fri, Jan 6, 2023 at 11:47 AM Bharath Rupireddy <[email protected]> wrote: > > On Thu, Jan 5, 2023 at 6:51 PM Bharath Rupireddy > <[email protected]> wrote: > > > > > I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that both > > > extracted images are the same and matches the one modified right after the checkpoint. > > > > > > What do you think? (could be done later in another patch though). > > > > I think pageinspect can be used here. We can fetch the raw page from > > the table after the checkpoint and raw FPI from the WAL record logged > > as part of the update. I've tried to do so [1], but I see a slight > > difference in the raw output. The expectation is that they both be the > > same. It might be that the update operation logs the FPI with some > > more info set (prune_xid). I'll try to see why it is so. > > > > I'm attaching the v2 patch for further review. > > > > [1] > > SELECT * FROM page_header(:'page_from_table'); > > lsn | checksum | flags | lower | upper | special | pagesize | > > version | prune_xid > > -----------+----------+-------+-------+-------+---------+----------+---------+----------- > > 0/1891D78 | 0 | 0 | 40 | 8064 | 8192 | 8192 | > > 4 | 0 > > (1 row) > > > > SELECT * FROM page_header(:'page_from_wal'); > > lsn | checksum | flags | lower | upper | special | pagesize | > > version | prune_xid > > -----------+----------+-------+-------+-------+---------+----------+---------+----------- > > 0/1891D78 | 0 | 0 | 44 | 8032 | 8192 | 8192 | > > 4 | 735 > > (1 row) > > Ugh, v2 patch missed the new file added, I'm attaching v3 patch for > further review. Sorry for the noise. I took a stab at how and what gets logged as FPI in WAL records: Option 1: WAL record with FPI contains both the unmodified table page from the disk after checkpoint and new tuple (not applied to the unmodified page) and the recovery (redo) applies the new tuple to the unmodified page as part of recovery. A bit more WAL is needed to store both unmodified page and new tuple data in the WAL record and recovery can get slower a bit too as it needs to stitch the modified page. Option 2: WAL record with FPI contains only the modified page (new tuple applied to the unmodified page from the disk after checkpoint) and the recovery (redo) just returns the applied block as BLK_RESTORED. Recovery can get faster with this approach and less WAL is needed to store just the modified page. My earlier understanding was that postgres does option (1), however, I was wrong, option (2) is what actually postgres has implemented for the obvious advantages specified. I now made the tests a bit stricter in checking the FPI contents (tuple values) pulled from the WAL record with raw page contents pulled from the table using the pageinspect extension. Please see the attached v4 patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com Attachments: [application/x-patch] v4-0001-Add-FPI-extract-function-to-pg_walinspect.patch (20.7K, ../../CALj2ACXesN9DTjgsekM8fig7CxhhxQfQP4fCiSJgcmp9wrZOvA@mail.gmail.com/2-v4-0001-Add-FPI-extract-function-to-pg_walinspect.patch) download | inline diff: From 6b577f15aede723f5cb8ea675d41d0efe9b96727 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Fri, 6 Jan 2023 17:07:49 +0000 Subject: [PATCH v4] Add FPI extract function to pg_walinspect --- contrib/pg_walinspect/Makefile | 3 +- .../pg_walinspect/expected/pg_walinspect.out | 84 ++++++++++++- contrib/pg_walinspect/meson.build | 1 + .../pg_walinspect/pg_walinspect--1.0--1.1.sql | 24 ++++ contrib/pg_walinspect/pg_walinspect.c | 110 ++++++++++++++++++ contrib/pg_walinspect/pg_walinspect.control | 2 +- contrib/pg_walinspect/sql/pg_walinspect.sql | 58 ++++++++- doc/src/sgml/pgwalinspect.sgml | 49 ++++++++ 8 files changed, 327 insertions(+), 4 deletions(-) create mode 100644 contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql diff --git a/contrib/pg_walinspect/Makefile b/contrib/pg_walinspect/Makefile index 960530eb6c..7412307ede 100644 --- a/contrib/pg_walinspect/Makefile +++ b/contrib/pg_walinspect/Makefile @@ -7,11 +7,12 @@ OBJS = \ PGFILEDESC = "pg_walinspect - functions to inspect contents of PostgreSQL Write-Ahead Log" EXTENSION = pg_walinspect -DATA = pg_walinspect--1.0.sql +DATA = pg_walinspect--1.0.sql pg_walinspect--1.0--1.1.sql REGRESS = pg_walinspect REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_walinspect/walinspect.conf +EXTRA_INSTALL = contrib/pageinspect # Disabled because these tests require "wal_level=replica", which # some installcheck users do not have (e.g. buildfarm clients). diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out index a1ee743457..b4ae466edb 100644 --- a/contrib/pg_walinspect/expected/pg_walinspect.out +++ b/contrib/pg_walinspect/expected/pg_walinspect.out @@ -1,4 +1,5 @@ CREATE EXTENSION pg_walinspect; +CREATE EXTENSION pageinspect; -- Make sure checkpoints don't interfere with the test. SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, false); ?column? @@ -10,7 +11,7 @@ CREATE TABLE sample_tbl(col1 int, col2 int); SELECT pg_current_wal_lsn() AS wal_lsn1 \gset INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2); SELECT pg_current_wal_lsn() AS wal_lsn2 \gset -INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2); +INSERT INTO sample_tbl SELECT * FROM generate_series(3, 4); -- =================================================================== -- Tests for input validation -- =================================================================== @@ -73,6 +74,62 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2' t (1 row) +-- =================================================================== +-- Tests to get full page image (FPI) from WAL record and compare it +-- with raw page (post-update) from table +-- =================================================================== +SELECT pg_current_wal_lsn() AS wal_lsn3 \gset +-- Force FPI on the next update +CHECKPOINT; +-- Update table to generate an FPI. We intentionally update a single row +-- here to ensure the predictability in verifying the FPI with raw page +-- content from the table. +UPDATE sample_tbl SET col1 = col1 * 100 WHERE col1 = 1; +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset +-- Get FPI from WAL record +SELECT fpi AS page_from_wal FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid' \gset +SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid'; + ok +---- + t +(1 row) + +-- Get raw page from table +SELECT get_raw_page('sample_tbl', 0) AS page_from_table \gset +-- Compare FPI from WAL record and page from table, they must be same +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_table'); + tuple_data_split +---------------------- + {"\\x01000000",NULL} + {"\\x02000000",NULL} + {"\\x03000000",NULL} + {"\\x04000000",NULL} + {"\\x64000000",NULL} +(5 rows) + +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_wal'); + tuple_data_split +---------------------- + {"\\x01000000",NULL} + {"\\x02000000",NULL} + {"\\x03000000",NULL} + {"\\x04000000",NULL} + {"\\x64000000",NULL} +(5 rows) + +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_table') +EXCEPT +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_wal'); + tuple_data_split +------------------ +(0 rows) + -- =================================================================== -- Tests for permissions -- =================================================================== @@ -98,6 +155,13 @@ SELECT has_function_privilege('regress_pg_walinspect', f (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no + has_function_privilege +------------------------ + f +(1 row) + -- Functions accessible by users with role pg_read_server_files GRANT pg_read_server_files TO regress_pg_walinspect; SELECT has_function_privilege('regress_pg_walinspect', @@ -121,6 +185,13 @@ SELECT has_function_privilege('regress_pg_walinspect', t (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + REVOKE pg_read_server_files FROM regress_pg_walinspect; -- Superuser can grant execute to other users GRANT EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) @@ -129,6 +200,8 @@ GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) TO regress_pg_walinspect; GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO regress_pg_walinspect; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + TO regress_pg_walinspect; SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes has_function_privilege @@ -150,12 +223,21 @@ SELECT has_function_privilege('regress_pg_walinspect', t (1 row) +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + has_function_privilege +------------------------ + t +(1 row) + REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) FROM regress_pg_walinspect; REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) FROM regress_pg_walinspect; REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM regress_pg_walinspect; +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + FROM regress_pg_walinspect; -- =================================================================== -- Clean up -- =================================================================== diff --git a/contrib/pg_walinspect/meson.build b/contrib/pg_walinspect/meson.build index f6e912a7d8..bf7b79b1b7 100644 --- a/contrib/pg_walinspect/meson.build +++ b/contrib/pg_walinspect/meson.build @@ -19,6 +19,7 @@ contrib_targets += pg_walinspect install_data( 'pg_walinspect.control', 'pg_walinspect--1.0.sql', + 'pg_walinspect--1.0--1.1.sql', kwargs: contrib_data_args, ) diff --git a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql new file mode 100644 index 0000000000..4ffbd91728 --- /dev/null +++ b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql @@ -0,0 +1,24 @@ +/* contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION pg_walinspect UPDATE TO '1.1'" to load this file. \quit + +-- +-- pg_get_wal_fpi_info() +-- +CREATE FUNCTION pg_get_wal_fpi_info(IN start_lsn pg_lsn, + IN end_lsn pg_lsn, + OUT lsn pg_lsn, + OUT tablespace_oid oid, + OUT database_oid oid, + OUT relfile_number oid, + OUT block_number int8, + OUT fork_name text, + OUT fpi bytea +) +RETURNS SETOF record +AS 'MODULE_PATHNAME', 'pg_get_wal_fpi_info' +LANGUAGE C STRICT PARALLEL SAFE; + +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) TO pg_read_server_files; diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c index 08a4c2fb52..8f992960e8 100644 --- a/contrib/pg_walinspect/pg_walinspect.c +++ b/contrib/pg_walinspect/pg_walinspect.c @@ -35,6 +35,7 @@ PG_FUNCTION_INFO_V1(pg_get_wal_records_info); PG_FUNCTION_INFO_V1(pg_get_wal_records_info_till_end_of_wal); PG_FUNCTION_INFO_V1(pg_get_wal_stats); PG_FUNCTION_INFO_V1(pg_get_wal_stats_till_end_of_wal); +PG_FUNCTION_INFO_V1(pg_get_wal_fpi_info); static bool IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn); static XLogReaderState *InitXLogReaderState(XLogRecPtr lsn); @@ -55,6 +56,7 @@ static void FillXLogStatsRow(const char *name, uint64 n, uint64 total_count, Datum *values, bool *nulls, uint32 ncols); static void GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn, bool stats_per_record); +static void GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record); /* * Check if the given LSN is in future. Also, return the LSN up to which the @@ -616,3 +618,111 @@ pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } + +/* + * Get full page images and their info associated with a given WAL record. + * Decompression is applied to the blocks, if necessary. + */ +static void +GetWALFPIInfo(FunctionCallInfo fcinfo, XLogReaderState *record) +{ +#define PG_GET_WAL_FPI_INFO_COLS 7 + int block_id; + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + + for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++) + { + PGAlignedBlock buf; + Page page; + bytea *raw_page; + BlockNumber blk; + RelFileLocator rnode; + ForkNumber fork; + Datum values[PG_GET_WAL_FPI_INFO_COLS] = {0}; + bool nulls[PG_GET_WAL_FPI_INFO_COLS] = {0}; + int i = 0; + + if (!XLogRecHasBlockRef(record, block_id)) + continue; + + if (!XLogRecHasBlockImage(record, block_id)) + continue; + + page = (Page) buf.data; + + if (!RestoreBlockImage(record, block_id, page)) + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg_internal("%s", record->errormsg_buf))); + + /* Full page exists, so let's output its info and content. */ + (void) XLogRecGetBlockTagExtended(record, block_id, + &rnode, &fork, &blk, NULL); + + values[i++] = LSNGetDatum(record->ReadRecPtr); + values[i++] = ObjectIdGetDatum(rnode.spcOid); + values[i++] = ObjectIdGetDatum(rnode.dbOid); + values[i++] = ObjectIdGetDatum(rnode.relNumber); + values[i++] = Int64GetDatum((int64) blk); + + if (fork >= 0 && fork <= MAX_FORKNUM) + values[i++] = CStringGetTextDatum(forkNames[fork]); + else + ereport(ERROR, + (errcode(ERRCODE_INTERNAL_ERROR), + errmsg_internal("invalid fork number: %u", fork))); + + /* Initialize bytea buffer to copy the FPI to. */ + raw_page = (bytea *) palloc(BLCKSZ + VARHDRSZ); + SET_VARSIZE(raw_page, BLCKSZ + VARHDRSZ); + + /* Take a verbatim copy of the FPI. */ + memcpy(VARDATA(raw_page), page, BLCKSZ); + + values[i++] = PointerGetDatum(raw_page); + + Assert(i == PG_GET_WAL_FPI_INFO_COLS); + + tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, + values, nulls); + } + +#undef PG_GET_WAL_FPI_INFO_COLS +} + +/* + * Get full page images and their info of all WAL records between start LSN and + * end LSN. Decompression is applied to the blocks, if necessary. + * + * This function emits an error if a future start or end WAL LSN i.e. WAL LSN + * the database system doesn't know about is specified. + */ +Datum +pg_get_wal_fpi_info(PG_FUNCTION_ARGS) +{ + XLogRecPtr start_lsn; + XLogRecPtr end_lsn; + XLogReaderState *xlogreader; + + start_lsn = PG_GETARG_LSN(0); + end_lsn = PG_GETARG_LSN(1); + + end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn); + + InitMaterializedSRF(fcinfo, 0); + + xlogreader = InitXLogReaderState(start_lsn); + + while (ReadNextXLogRecord(xlogreader) && + xlogreader->EndRecPtr <= end_lsn) + { + GetWALFPIInfo(fcinfo, xlogreader); + + CHECK_FOR_INTERRUPTS(); + } + + pfree(xlogreader->private_data); + XLogReaderFree(xlogreader); + + PG_RETURN_VOID(); +} diff --git a/contrib/pg_walinspect/pg_walinspect.control b/contrib/pg_walinspect/pg_walinspect.control index 017e56a2bb..efa3cb2cfe 100644 --- a/contrib/pg_walinspect/pg_walinspect.control +++ b/contrib/pg_walinspect/pg_walinspect.control @@ -1,5 +1,5 @@ # pg_walinspect extension comment = 'functions to inspect contents of PostgreSQL Write-Ahead Log' -default_version = '1.0' +default_version = '1.1' module_pathname = '$libdir/pg_walinspect' relocatable = true diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql index 1b265ea7bc..6b4c3dd90a 100644 --- a/contrib/pg_walinspect/sql/pg_walinspect.sql +++ b/contrib/pg_walinspect/sql/pg_walinspect.sql @@ -1,5 +1,7 @@ CREATE EXTENSION pg_walinspect; +CREATE EXTENSION pageinspect; + -- Make sure checkpoints don't interfere with the test. SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot', true, false); @@ -11,7 +13,7 @@ INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2); SELECT pg_current_wal_lsn() AS wal_lsn2 \gset -INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2); +INSERT INTO sample_tbl SELECT * FROM generate_series(3, 4); -- =================================================================== -- Tests for input validation @@ -52,6 +54,45 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2' SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2') WHERE resource_manager = 'Heap' AND record_type = 'INSERT'; +-- =================================================================== +-- Tests to get full page image (FPI) from WAL record and compare it +-- with raw page (post-update) from table +-- =================================================================== +SELECT pg_current_wal_lsn() AS wal_lsn3 \gset + +-- Force FPI on the next update +CHECKPOINT; + +-- Update table to generate an FPI. We intentionally update a single row +-- here to ensure the predictability in verifying the FPI with raw page +-- content from the table. +UPDATE sample_tbl SET col1 = col1 * 100 WHERE col1 = 1; + +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset + +-- Get FPI from WAL record +SELECT fpi AS page_from_wal FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid' \gset + +SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid'; + +-- Get raw page from table +SELECT get_raw_page('sample_tbl', 0) AS page_from_table \gset + +-- Compare FPI from WAL record and page from table, they must be same +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_table'); + +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_wal'); + +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_table') +EXCEPT +SELECT tuple_data_split('sample_tbl'::regclass, t_data, t_infomask, t_infomask2, t_bits) + FROM heap_page_items(:'page_from_wal'); + -- =================================================================== -- Tests for permissions -- =================================================================== @@ -66,6 +107,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- no +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no + -- Functions accessible by users with role pg_read_server_files GRANT pg_read_server_files TO regress_pg_walinspect; @@ -79,6 +123,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + REVOKE pg_read_server_files FROM regress_pg_walinspect; -- Superuser can grant execute to other users @@ -91,6 +138,9 @@ GRANT EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) GRANT EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) TO regress_pg_walinspect; +GRANT EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + TO regress_pg_walinspect; + SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes @@ -100,6 +150,9 @@ SELECT has_function_privilege('regress_pg_walinspect', SELECT has_function_privilege('regress_pg_walinspect', 'pg_get_wal_stats(pg_lsn, pg_lsn, boolean) ', 'EXECUTE'); -- yes +SELECT has_function_privilege('regress_pg_walinspect', + 'pg_get_wal_fpi_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- yes + REVOKE EXECUTE ON FUNCTION pg_get_wal_record_info(pg_lsn) FROM regress_pg_walinspect; @@ -109,6 +162,9 @@ REVOKE EXECUTE ON FUNCTION pg_get_wal_records_info(pg_lsn, pg_lsn) REVOKE EXECUTE ON FUNCTION pg_get_wal_stats(pg_lsn, pg_lsn, boolean) FROM regress_pg_walinspect; +REVOKE EXECUTE ON FUNCTION pg_get_wal_fpi_info(pg_lsn, pg_lsn) + FROM regress_pg_walinspect; + -- =================================================================== -- Clean up -- =================================================================== diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml index 1a1bee7d6a..ef5e5852a9 100644 --- a/doc/src/sgml/pgwalinspect.sgml +++ b/doc/src/sgml/pgwalinspect.sgml @@ -261,6 +261,55 @@ postgres=# select * from pg_get_wal_stats('0/14AFC30', '0/15011D7', true) where </listitem> </varlistentry> + <varlistentry> + <term> + <function> + pg_get_wal_fpi_info(start_lsn pg_lsn, + end_lsn pg_lsn, + lsn OUT pg_lsn, + tablespace_oid OUT oid, + database_oid OUT oid, + relfile_number OUT oid, + block_number OUT int8, + fork_name OUT text, + fpi OUT bytea) + returns setof record + </function> + </term> + + <listitem> + <para> + Gets raw full page images and their information associated with all the + valid WAL records between <replaceable>start_lsn</replaceable> and + <replaceable>end_lsn</replaceable>. Returns one row per full page image. + Decompression is applied to the blocks, if necessary. + If <replaceable>start_lsn</replaceable> or + <replaceable>end_lsn</replaceable> are not yet available, the function + will raise an error. For example, usage of the function is as follows: +<screen> +postgres=# SELECT lsn, tablespace_oid, database_oid, relfile_number, block_number, fork_name, length(fpi) > 0 as fpi_ok FROM pg_get_wal_fpi_info('0/7418E60', '0/7518218'); + lsn | tablespace_oid | database_oid | relfile_number | block_number | fork_name | fpi_ok +-----------+----------------+--------------+----------------+--------------+-----------+-------- + 0/7418F48 | 1663 | 5 | 16419 | 0 | main | t + 0/7419080 | 1663 | 5 | 16422 | 19 | main | t + 0/741B0B8 | 1663 | 5 | 16422 | 22 | main | t + 0/741BF48 | 1663 | 5 | 16422 | 20 | main | t + 0/74257B8 | 1663 | 5 | 16422 | 21 | main | t + 0/742FF30 | 1663 | 5 | 16425 | 42 | main | t + 0/7431F68 | 1663 | 5 | 16425 | 44 | main | t + 0/7439B78 | 1663 | 5 | 16425 | 43 | main | t + 0/74451E8 | 1663 | 5 | 16428 | 199 | main | t + 0/7447220 | 1663 | 5 | 16428 | 221 | main | t + 0/744E458 | 1663 | 5 | 16428 | 200 | main | t + 0/7457CB0 | 1663 | 5 | 16428 | 201 | main | t + 0/7461520 | 1663 | 5 | 16428 | 202 | main | t + 0/746AD90 | 1663 | 5 | 16428 | 203 | main | t +(14 rows) +</screen> + </para> + </listitem> + </varlistentry> + </variablelist> </sect2> -- 2.34.1 ^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Add a new pg_walinspect function to extract FPIs from WAL records @ 2023-01-10 08:29 Drouvot, Bertrand <[email protected]> parent: Bharath Rupireddy <[email protected]> 0 siblings, 1 reply; 24+ messages in thread From: Drouvot, Bertrand @ 2023-01-10 08:29 UTC (permalink / raw) To: Bharath Rupireddy <[email protected]>; PostgreSQL Hackers <[email protected]> Hi, On 1/6/23 6:41 PM, Bharath Rupireddy wrote: > On Fri, Jan 6, 2023 at 11:47 AM Bharath Rupireddy > <[email protected]> wrote: >> >> On Thu, Jan 5, 2023 at 6:51 PM Bharath Rupireddy >> <[email protected]> wrote: >>> >>>> I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that both >>>> extracted images are the same and matches the one modified right after the checkpoint. >>>> >>>> What do you think? (could be done later in another patch though). >>> >>> I think pageinspect can be used here. We can fetch the raw page from >>> the table after the checkpoint and raw FPI from the WAL record logged >>> as part of the update. I've tried to do so [1], but I see a slight >>> difference in the raw output. The expectation is that they both be the >>> same. It might be that the update operation logs the FPI with some >>> more info set (prune_xid). I'll try to see why it is so. >>> >>> I'm attaching the v2 patch for further review. >>> >>> [1] >>> SELECT * FROM page_header(:'page_from_table'); >>> lsn | checksum | flags | lower | upper | special | pagesize | >>> version | prune_xid >>> -----------+----------+-------+-------+-------+---------+----------+---------+----------- >>> 0/1891D78 | 0 | 0 | 40 | 8064 | 8192 | 8192 | >>> 4 | 0 >>> (1 row) >>> >>> SELECT * FROM page_header(:'page_from_wal'); >>> lsn | checksum | flags | lower | upper | special | pagesize | >>> version | prune_xid >>> -----------+----------+-------+-------+-------+---------+----------+---------+----------- >>> 0/1891D78 | 0 | 0 | 44 | 8032 | 8192 | 8192 | >>> 4 | 735 >>> (1 row) >> >> Ugh, v2 patch missed the new file added, I'm attaching v3 patch for >> further review. Sorry for the noise. > > I took a stab at how and what gets logged as FPI in WAL records: > > Option 1: > WAL record with FPI contains both the unmodified table page from the > disk after checkpoint and new tuple (not applied to the unmodified > page) and the recovery (redo) applies the new tuple to the unmodified > page as part of recovery. A bit more WAL is needed to store both > unmodified page and new tuple data in the WAL record and recovery can > get slower a bit too as it needs to stitch the modified page. > > Option 2: > WAL record with FPI contains only the modified page (new tuple applied > to the unmodified page from the disk after checkpoint) and the > recovery (redo) just returns the applied block as BLK_RESTORED. > Recovery can get faster with this approach and less WAL is needed to > store just the modified page. > > My earlier understanding was that postgres does option (1), however, I > was wrong, option (2) is what actually postgres has implemented for > the obvious advantages specified. > > I now made the tests a bit stricter in checking the FPI contents > (tuple values) pulled from the WAL record with raw page contents > pulled from the table using the pageinspect extension. Please see the > attached v4 patch. > Thanks for updating the patch! +-- Compare FPI from WAL record and page from table, they must be same I think "must be the same" or "must be identical" sounds better (but not 100% sure). Except this nit, V4 looks good to me. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: Add a new pg_walinspect function to extract FPIs from WAL records @ 2023-01-11 04:37 Michael Paquier <[email protected]> parent: Drouvot, Bertrand <[email protected]> 0 siblings, 0 replies; 24+ messages in thread From: Michael Paquier @ 2023-01-11 04:37 UTC (permalink / raw) To: Drouvot, Bertrand <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Jan 10, 2023 at 09:29:03AM +0100, Drouvot, Bertrand wrote: > Thanks for updating the patch! > > +-- Compare FPI from WAL record and page from table, they must be same > > I think "must be the same" or "must be identical" sounds better (but not 100% sure). > > Except this nit, V4 looks good to me. +postgres=# SELECT lsn, tablespace_oid, database_oid, relfile_number, block_number, fork_name, length(fpi) > 0 as fpi_ok FROM pg_get_wal_fpi_info('0/7418E60', '0/7518218'); This query in the docs is too long IMO. Could you split that across multiple lines for readability? + pg_get_wal_fpi_info(start_lsn pg_lsn, + end_lsn pg_lsn, + lsn OUT pg_lsn, + tablespace_oid OUT oid, + database_oid OUT oid, + relfile_number OUT oid, + block_number OUT int8, + fork_name OUT text, + fpi OUT bytea) I am a bit surprised by this format, used to define the functions part of the module in the docs, while we have examples that actually show what's printed out. I understand that this comes from the original commit of the module, but the rendered docs are really hard to parse as well, no? FWIW, I think that this had better be fixed as well in the docs of v15.. Showing a full set of attributes for the returned record is fine by me, still if these are too long we could just use \x. For this one, I think that there is little point in showing 14 records, so I would stick with a style similar to pageinspect. +CREATE FUNCTION pg_get_wal_fpi_info(IN start_lsn pg_lsn, + IN end_lsn pg_lsn, + OUT lsn pg_lsn, + OUT tablespace_oid oid, Slight indentation issue here. Using "relfile_number" would be a first, for what is defined in the code and the docs as a filenode. +SELECT pg_current_wal_lsn() AS wal_lsn4 \gset +-- Get FPI from WAL record +SELECT fpi AS page_from_wal FROM pg_get_wal_fpi_info(:'wal_lsn3', :'wal_lsn4') + WHERE relfile_number = :'sample_tbl_oid' \gset I would be tempted to keep the checks run here minimal with only a basic set of checks on the LSN, without the dependencies to pageinspect (tuple_data_split and get_raw_page), which would be fine enough to check the execution of the function. FWIW, I am surprised by the design choice behind ValidateInputLSNs() to allow data to be gathered until the end of WAL in some cases, but to not allow it in others. It is likely too late to come back to this choice for the existing functions in v15 (quoique?), but couldn't it be useful to make this new FPI function work at least with an insanely high LSN value to make sure that we fetch all the FPIs from a given start position, up to the end of WAL? That looks like a pretty good default behavior to me, rather than issuing an error when a LSN is defined as in the future.. I am really wondering why we have ValidateInputLSNs(till_end_of_wal=false) to begin with, while we could just allow any LSN value in the future automatically, as we can know the current insert or replay LSNs (depending on the recovery state). -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../Y749H%[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 24+ messages in thread
end of thread, other threads:[~2023-01-11 04:37 UTC | newest] Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-12-13 06:07 [PATCH v5 6/8] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v20 3/4] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v19 3/4] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v18 3/4] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v9 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v14 3/3] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v10 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v17 3/4] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v4 6/8] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v15 3/4] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v7 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v6 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v12 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v10 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v8 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v13 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v11 6/6] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2021-12-13 06:07 [PATCH v16 3/4] Move removal of old logical rewrite mapping files to custodian. Nathan Bossart <[email protected]> 2023-01-05 13:21 Re: Add a new pg_walinspect function to extract FPIs from WAL records Bharath Rupireddy <[email protected]> 2023-01-06 05:54 ` Re: Add a new pg_walinspect function to extract FPIs from WAL records vignesh C <[email protected]> 2023-01-06 06:17 ` Re: Add a new pg_walinspect function to extract FPIs from WAL records Bharath Rupireddy <[email protected]> 2023-01-06 17:41 ` Re: Add a new pg_walinspect function to extract FPIs from WAL records Bharath Rupireddy <[email protected]> 2023-01-10 08:29 ` Re: Add a new pg_walinspect function to extract FPIs from WAL records Drouvot, Bertrand <[email protected]> 2023-01-11 04:37 ` Re: Add a new pg_walinspect function to extract FPIs from WAL records 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