Hi, > > Here is an updated patch. The steps to test it manually are as follows. > > > > Compile and install PostgreSQL from the REL_17_STABLE branch: > > > > [...] > > > > As always, your feedback and suggestions are most welcomed. > > Rebased.
Rebased. -- Best regards, Aleksander Alekseev
From 43e7cdccf1aa96e6522a41518bf0437dda23cc4d Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <aleksander@timescale.com> Date: Wed, 11 Sep 2024 13:17:33 +0300 Subject: [PATCH v4] Always use long SLRU segment file names PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used short or long file names depending on SlruCtl->long_segment_names. This commit refactors SLRU to always use long file names in order to simplify the code. Aleksander Alekseev, reviewed by Michael Paquier Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com (!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h --- src/backend/access/transam/clog.c | 2 +- src/backend/access/transam/commit_ts.c | 3 +- src/backend/access/transam/multixact.c | 6 +- src/backend/access/transam/slru.c | 73 ++++---------------- src/backend/access/transam/subtrans.c | 2 +- src/backend/commands/async.c | 2 +- src/backend/storage/lmgr/predicate.c | 2 +- src/bin/pg_upgrade/pg_upgrade.c | 74 +++++++++++++++++++++ src/bin/pg_upgrade/pg_upgrade.h | 6 ++ src/bin/pg_verifybackup/t/003_corruption.pl | 2 +- src/include/access/slru.h | 10 +-- src/test/modules/test_slru/test_slru.c | 8 +-- 12 files changed, 104 insertions(+), 86 deletions(-) diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index 48f10bec91e..f130403ac4b 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -810,7 +810,7 @@ CLOGShmemInit(void) XactCtl->PagePrecedes = CLOGPagePrecedes; SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE, "pg_xact", LWTRANCHE_XACT_BUFFER, - LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false); + LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG); SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE); } diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c index 113fae1437a..59535526823 100644 --- a/src/backend/access/transam/commit_ts.c +++ b/src/backend/access/transam/commit_ts.c @@ -556,8 +556,7 @@ CommitTsShmemInit(void) SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0, "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER, LWTRANCHE_COMMITTS_SLRU, - SYNC_HANDLER_COMMIT_TS, - false); + SYNC_HANDLER_COMMIT_TS); SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE); commitTsShared = ShmemInitStruct("CommitTs shared", diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index c1e2c42e1bb..b3b11c1a0ad 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -1974,15 +1974,13 @@ MultiXactShmemInit(void) "multixact_offset", multixact_offset_buffers, 0, "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER, LWTRANCHE_MULTIXACTOFFSET_SLRU, - SYNC_HANDLER_MULTIXACT_OFFSET, - false); + SYNC_HANDLER_MULTIXACT_OFFSET); SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE); SimpleLruInit(MultiXactMemberCtl, "multixact_member", multixact_member_buffers, 0, "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER, LWTRANCHE_MULTIXACTMEMBER_SLRU, - SYNC_HANDLER_MULTIXACT_MEMBER, - false); + SYNC_HANDLER_MULTIXACT_MEMBER); /* doesn't call SimpleLruTruncate() or meet criteria for unit tests */ /* Initialize our shared state struct */ diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index 9ce628e62a5..cc078d2e6ce 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -77,42 +77,22 @@ * * "path" should point to a buffer at least MAXPGPATH characters long. * - * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1]. - * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF. - * - * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1]. - * The resulting file name is made of 4 to 6 characters, as of: - * - * dir/1234 for [0, 2^16-1] - * dir/12345 for [2^16, 2^20-1] - * dir/123456 for [2^20, 2^24-1] + * segno can be in the range [0, 2^60-1]. The resulting file name is made + * of 15 characters, e.g. dir/123456789ABCDEF. */ static inline int SlruFileName(SlruCtl ctl, char *path, int64 segno) { - if (ctl->long_segment_names) - { - /* - * We could use 16 characters here but the disadvantage would be that - * the SLRU segments will be hard to distinguish from WAL segments. - * - * For this reason we use 15 characters. It is enough but also means - * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily. - */ - Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF)); - return snprintf(path, MAXPGPATH, "%s/%015llX", ctl->Dir, - (long long) segno); - } - else - { - /* - * Despite the fact that %04X format string is used up to 24 bit - * integers are allowed. See SlruCorrectSegmentFilenameLength() - */ - Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF)); - return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir, - (unsigned int) segno); - } + /* + * We could use 16 characters here but the disadvantage would be that + * the SLRU segments will be hard to distinguish from WAL segments. + * + * For this reason we use 15 characters. It is enough but also means + * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily. + */ + Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF)); + return snprintf(path, MAXPGPATH, "%s/%015llX", ctl->Dir, + (long long) segno); } /* @@ -251,7 +231,7 @@ SimpleLruAutotuneBuffers(int divisor, int max) void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns, const char *subdir, int buffer_tranche_id, int bank_tranche_id, - SyncRequestHandler sync_handler, bool long_segment_names) + SyncRequestHandler sync_handler) { SlruShared shared; bool found; @@ -342,7 +322,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns, */ ctl->shared = shared; ctl->sync_handler = sync_handler; - ctl->long_segment_names = long_segment_names; ctl->nbanks = nbanks; strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir)); } @@ -1748,30 +1727,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data) return false; /* keep going */ } -/* - * An internal function used by SlruScanDirectory(). - * - * Returns true if a file with a name of a given length may be a correct - * SLRU segment. - */ -static inline bool -SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len) -{ - if (ctl->long_segment_names) - return (len == 15); /* see SlruFileName() */ - else - - /* - * Commit 638cf09e76d allowed 5-character lengths. Later commit - * 73c986adde5 allowed 6-character length. - * - * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page - * numbers, and the corresponding 15-character file names, which may - * eventually deprecate the support for 4, 5, and 6-character names. - */ - return (len == 4 || len == 5 || len == 6); -} - /* * Scan the SimpleLru directory and apply a callback to each file found in it. * @@ -1803,7 +1758,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data) len = strlen(clde->d_name); - if (SlruCorrectSegmentFilenameLength(ctl, len) && + if ((len == 15) && strspn(clde->d_name, "0123456789ABCDEF") == len) { segno = strtoi64(clde->d_name, NULL, 16); diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index 15153618fad..58a5ef657ea 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -243,7 +243,7 @@ SUBTRANSShmemInit(void) SubTransCtl->PagePrecedes = SubTransPagePrecedes; SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0, "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER, - LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false); + LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE); SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE); } diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 4bd37d5beb5..373b0357fad 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -537,7 +537,7 @@ AsyncShmemInit(void) NotifyCtl->PagePrecedes = asyncQueuePagePrecedes; SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0, "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU, - SYNC_HANDLER_NONE, true); + SYNC_HANDLER_NONE); if (!found) { diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 5b21a053981..bc83e8e859d 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -814,7 +814,7 @@ SerialInit(void) SimpleLruInit(SerialSlruCtl, "serializable", serializable_buffers, 0, "pg_serial", LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU, - SYNC_HANDLER_NONE, false); + SYNC_HANDLER_NONE); #ifdef USE_ASSERT_CHECKING SerialPagePrecedesLogicallyUnitTests(); #endif diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index d95c491fb57..8b10908e1f2 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -38,6 +38,7 @@ #include "postgres_fe.h" +#include <dirent.h> #include <time.h> #include "catalog/pg_class_d.h" @@ -60,6 +61,8 @@ static void prepare_new_cluster(void); static void prepare_new_globals(void); static void create_new_objects(void); static void copy_xact_xlog_xid(void); +static void check_slru_segment_filenames(void); +static void rename_slru_segments(const char *dirname); static void set_frozenxids(bool minmxid_only); static void make_outputdirs(char *pgdata); static void setup(char *argv0); @@ -156,6 +159,7 @@ main(int argc, char **argv) copy_xact_xlog_xid(); set_new_cluster_char_signedness(); + check_slru_segment_filenames(); /* New now using xids of the old system */ @@ -840,6 +844,76 @@ copy_xact_xlog_xid(void) check_ok(); } +static void +rename_slru_segments(const char* dirname) +{ + DIR *dir; + struct dirent *de; + int len; + int64 segno; + char dir_path[MAXPGPATH]; + char old_path[MAXPGPATH]; + char new_path[MAXPGPATH]; + + prep_status("Renaming SLRU segments in %s", dirname); + snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname); + + dir = opendir(dir_path); + if (dir == NULL) + pg_fatal("could not open directory \"%s\": %m", dir_path); + + while (errno = 0, (de = readdir(dir)) != NULL) + { + /* + * ignore '.', '..' and everything else that doesn't look + * like an SLRU segment with a short file name + */ + + len = strlen(de->d_name); + if(len != 4 && len != 5 && len != 6) + continue; + + if(strspn(de->d_name, "0123456789ABCDEF") != len) + continue; + + segno = strtoi64(de->d_name, NULL, 16); + snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path, + (long long) segno); + snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name); + + if (pg_mv_file(old_path, new_path) != 0) + pg_fatal("could not rename file \"%s\" to \"%s\": %m", + old_path, new_path); + } + + if (errno) + pg_fatal("could not read directory \"%s\": %m", dir_path); + + if (closedir(dir)) + pg_fatal("could not close directory \"%s\": %m", dir_path); + + check_ok(); +} + +static void +check_slru_segment_filenames(void) +{ + int i; + static const char* dirs[] = { + "pg_xact", + "pg_commit_ts", + "pg_multixact/offsets", + "pg_multixact/members", + "pg_subtrans", + "pg_serial", + }; + + if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER) + return; + + for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++) + rename_slru_segments(dirs[i]); +} /* * set_frozenxids() diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index f4e375d27c7..0340a20a391 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -131,6 +131,12 @@ extern char *output_files[]; */ #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212 +/* + * change of SLRU segment filenames length in 18.0 + * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING + */ +#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202412201 + /* * Each relation is represented by a relinfo structure. */ diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl index 8ef7f8a4e7a..367212af529 100644 --- a/src/bin/pg_verifybackup/t/003_corruption.pl +++ b/src/bin/pg_verifybackup/t/003_corruption.pl @@ -246,7 +246,7 @@ sub mutilate_extra_tablespace_file sub mutilate_missing_file { my ($backup_path) = @_; - my $pathname = "$backup_path/pg_xact/0000"; + my $pathname = "$backup_path/pg_xact/000000000000000"; unlink($pathname) || die "$pathname: $!"; return; } diff --git a/src/include/access/slru.h b/src/include/access/slru.h index e142800aab2..ecf2ca79692 100644 --- a/src/include/access/slru.h +++ b/src/include/access/slru.h @@ -131,13 +131,6 @@ typedef struct SlruCtlData /* Number of banks in this SLRU. */ uint16 nbanks; - /* - * If true, use long segment file names. Otherwise, use short file names. - * - * For details about the file name format, see SlruFileName(). - */ - bool long_segment_names; - /* * Which sync handler function to use when handing sync requests over to * the checkpointer. SYNC_HANDLER_NONE to disable fsync (eg pg_notify). @@ -184,8 +177,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns); extern int SimpleLruAutotuneBuffers(int divisor, int max); extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns, const char *subdir, int buffer_tranche_id, - int bank_tranche_id, SyncRequestHandler sync_handler, - bool long_segment_names); + int bank_tranche_id, SyncRequestHandler sync_handler); extern int SimpleLruZeroPage(SlruCtl ctl, int64 pageno); extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok, TransactionId xid); diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 3ea5ceb8552..cbd5173015a 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2) static void test_slru_shmem_startup(void) { - /* - * Short segments names are well tested elsewhere so in this test we are - * focusing on long names. - */ - const bool long_segment_names = true; const char slru_dir_name[] = "pg_test_slru"; int test_tranche_id; int test_buffer_tranche_id; @@ -241,8 +236,7 @@ test_slru_shmem_startup(void) TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", NUM_TEST_BUFFERS, 0, slru_dir_name, - test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE, - long_segment_names); + test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE); } void -- 2.48.1
