[PATCH] Refactor SLRU to always use long file names
Hi,
Commit 4ed8f0913bfd introduced long SLRU file names. The proposed
patch removes SlruCtl->long_segment_names flag and makes SLRU always
use long file names. This simplifies both the code and the API.
Corresponding changes to pg_upgrade are included.
One drawback I see is that technically SLRU is an exposed API and
changing it may affect third-party code. I'm not sure if we should
seriously worry about this. Firstly, the change is trivial and
secondly, it's not clear whether such third-party code even exists (we
broke this API just recently in 4ed8f0913bfd and no one complained).
I didn't include any tests for the new pg_upgrade code. To my
knowledge we test it manually, with buildfarm members and during
alpha- and beta-testing periods. Please let me know if you think there
should be a corresponding TAP test.
Thoughts?
--
Best regards,
Aleksander Alekseev
Attachments:
v1-0001-Always-use-long-SLRU-segment-file-names.patchapplication/octet-stream; name=v1-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From c297163e9b69d6f978301eeb629834b73e81e2d4 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v1] 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 TODO FIXME
Discussion: TODO FIXME
---
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 | 78 +++++++++++++++++++++
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, 108 insertions(+), 86 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index e6f79320e9..71fa291c10 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 77e1899d7a..44de962a28 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 8c37d7eba7..4072a78f3c 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 e7f73bf427..d5cc5fc6e2 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->bank_mask = (nslots / SLRU_BANK_SIZE) - 1;
strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
}
@@ -1745,30 +1724,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.
*
@@ -1800,7 +1755,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 50bb1d8cfc..b92d7d4e27 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 8ed503e1c1..84e89e6f00 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 e24a0f2fdb..9e8a4c7851 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 663235816f..9a78ecc91a 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"
@@ -59,6 +60,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);
@@ -154,6 +157,7 @@ main(int argc, char **argv)
*/
copy_xact_xlog_xid();
+ check_slru_segment_filenames();
/* New now using xids of the old system */
@@ -806,6 +810,80 @@ 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",
+ };
+
+ /*
+ TODO FIXME UNCOMMENT BEFORE COMMITTING
+ if(new_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 cdb6e2b759..ba3ad0f490 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -125,6 +125,12 @@ extern char *output_files[];
*/
#define JSONB_FORMAT_CHANGE_CAT_VER 201409291
+/*
+ * 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 20241001
+
/*
* 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 ae91e04338..b4ea3a8bd9 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -182,7 +182,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 97e612cd10..94c2e3712d 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -133,13 +133,6 @@ typedef struct SlruCtlData
*/
bits16 bank_mask;
- /*
- * 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).
@@ -187,8 +180,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 d227b06703..de6a6bffd2 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.46.0
On Wed, Sep 11, 2024 at 04:07:06PM +0300, Aleksander Alekseev wrote:
Commit 4ed8f0913bfd introduced long SLRU file names. The proposed
patch removes SlruCtl->long_segment_names flag and makes SLRU always
use long file names. This simplifies both the code and the API.
Corresponding changes to pg_upgrade are included.
That's leaner, indeed.
One drawback I see is that technically SLRU is an exposed API and
changing it may affect third-party code. I'm not sure if we should
seriously worry about this. Firstly, the change is trivial and
secondly, it's not clear whether such third-party code even exists (we
broke this API just recently in 4ed8f0913bfd and no one complained).
Any third-party code using custom SLRUs would need to take care of
handling their upgrade path outside pg_upgrade. Not sure there are
any of them, TBH, but let's see.
I didn't include any tests for the new pg_upgrade code. To my
knowledge we test it manually, with buildfarm members and during
alpha- and beta-testing periods. Please let me know if you think there
should be a corresponding TAP test.
Removing the old API means that it is impossible to test a move from
short to long file names. That's OK by me to rely on the pg_upgrade
paths in the buildfarm code. We have a few of them.
There is one thing I am wondering, here, though, which is to think
harder about a validity check at the end of 002_pg_upgrade.pl to make
sure that all the SLRU use long file names after running the tests.
That would mean thinking about a mechanism to list all of them from a
backend, rather than hardcode a list of them. Perhaps that's not
worth it, just dropping an idea in the bucket of ideas. I would guess
in the shape of a catalog that's able to represent at SQL level all
the SLRUs that exist in a backend.
--
Michael
Hi Michael,
On Wed, Sep 11, 2024 at 04:07:06PM +0300, Aleksander Alekseev wrote:
Commit 4ed8f0913bfd introduced long SLRU file names. The proposed
patch removes SlruCtl->long_segment_names flag and makes SLRU always
use long file names. This simplifies both the code and the API.
Corresponding changes to pg_upgrade are included.That's leaner, indeed.
One drawback I see is that technically SLRU is an exposed API and
changing it may affect third-party code. I'm not sure if we should
seriously worry about this. Firstly, the change is trivial and
secondly, it's not clear whether such third-party code even exists (we
broke this API just recently in 4ed8f0913bfd and no one complained).Any third-party code using custom SLRUs would need to take care of
handling their upgrade path outside pg_upgrade. Not sure there are
any of them, TBH, but let's see.I didn't include any tests for the new pg_upgrade code. To my
knowledge we test it manually, with buildfarm members and during
alpha- and beta-testing periods. Please let me know if you think there
should be a corresponding TAP test.Removing the old API means that it is impossible to test a move from
short to long file names. That's OK by me to rely on the pg_upgrade
paths in the buildfarm code. We have a few of them.
Thanks for the feedback.
There is one thing I am wondering, here, though, which is to think
harder about a validity check at the end of 002_pg_upgrade.pl to make
sure that all the SLRU use long file names after running the tests.
That would mean thinking about a mechanism to list all of them from a
backend, rather than hardcode a list of them. Perhaps that's not
worth it, just dropping an idea in the bucket of ideas. I would guess
in the shape of a catalog that's able to represent at SQL level all
the SLRUs that exist in a backend.
Hmm... IMO it would be a rather niche facility to maintain in PG core.
At least I'm not aware of cases when a DBA wanted to list initialized
SLRUs. Would it be convenient for core / extensions developers?
Creating a breakpoint on SimpleLruInit() or adding a temporary elog()
sounds simpler to me.
It wouldn't hurt re-checking the segment file names in the TAP test
but this would mean hardcoding catalog names which as I understand you
want to avoid. With high probability PG wouldn't start if the
corresponding piece of pg_upgrade is wrong (I checked more than once
:). So I'm not entirely sure if it's worth the effort, but let's see
what others think.
--
Best regards,
Aleksander Alekseev
On Thu, Sep 12, 2024 at 12:33:14PM +0300, Aleksander Alekseev wrote:
It wouldn't hurt re-checking the segment file names in the TAP test
but this would mean hardcoding catalog names which as I understand you
want to avoid. With high probability PG wouldn't start if the
corresponding piece of pg_upgrade is wrong (I checked more than once
:). So I'm not entirely sure if it's worth the effort, but let's see
what others think.
+ 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);
Your patch is just doing a rename() of the files from short to long
names. How about adding a new TAP script in pg_upgrade that creates a
couple of empty files with short files names in each path that needs
to do the transfer? Then the test could run one pg_upgrade command
and check that the new names are in place. You could use a array of
objects, with the base path, the old name and the new name, then loop
over it. With the check in check_slru_segment_filenames() based on
SLRU_SEG_FILENAMES_CHANGE_CAT_VER, that should work.
+ static const char* dirs[] = {
+ "pg_xact",
+ "pg_commit_ts",
+ "pg_multixact/offsets",
+ "pg_multixact/members",
+ "pg_subtrans",
+ "pg_serial",
+ };
Hardcoding that is always annoying, but well, that's not going to
change. So living with this is not going to be a maintenance burden.
--
Michael
Hi Michael,
Thanks for your feedback!
Your patch is just doing a rename() of the files from short to long
names. How about adding a new TAP script in pg_upgrade that creates a
couple of empty files with short files names in each path that needs
to do the transfer? Then the test could run one pg_upgrade command
and check that the new names are in place. You could use a array of
objects, with the base path, the old name and the new name, then loop
over it. With the check in check_slru_segment_filenames() based on
SLRU_SEG_FILENAMES_CHANGE_CAT_VER, that should work.
OK I gave it a try and discovered that the test becomes very ugly very
fast. Attached is the draft of the test to give you an idea of how
it's going to look like.
In order to trigger renaming of SLRU segments first we have to
downgrade the catalog version in the pg_control file. Otherwise the
check in check_slru_segment_filenames() is not going to pass and
pg_upgrade will do nothing (*). This per se is easily done with
binmode() and pack() however the file has a CRC. Calculating it is not
difficult since we have pg_read_binary_file() and crc32c() SQL
functions, although personally I don't find a need for starting a
cluster for this quite satisfactory. The CRC is stored by the offset
`sizeof(ControlData)-4` and unless I'm wrong is platform-dependent.
I see several solutions for this problem:
* We could add sizeof(ControlData) to the output of `pg_controldata`
so we could use it from Perl
* We could teach `initdb` to override the catalog version
* We could implement a new tool for editing pg_control file
On top of that I should add that the test takes about 7 seconds on my
laptop. Apparently executing two initdb's and one pg_upgrade is not
very cheap. This makes me wonder if instead of writing a new test we
should modify 002_pg_upgrade.pl. This however will make the test even
less readable and maintainable.
What do you think?
(*) BTW I noticed a mistake in the commented code. The condition
should be `>=`, not `<`, i.e:
```
if(new_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
return;
```
--
Best regards,
Aleksander Alekseev
Attachments:
Hi again,
Just a quick follow-up.
(*) BTW I noticed a mistake in the commented code. The condition
should be `>=`, not `<`, i.e:```
if(new_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
return;
```
The concentration of caffeine in my blood is a bit low right now. I
suspect I may need to re-check this statement with a fresh head.
Also it occured to me that as a 4th option we could just get rid of
this check. Users however will pay the price every time they execute
pg_upgrade so I doubt we are going to do this.
--
Best regards,
Aleksander Alekseev
On Tue, Nov 12, 2024 at 05:37:02PM +0300, Aleksander Alekseev wrote:
Also it occured to me that as a 4th option we could just get rid of
this check. Users however will pay the price every time they execute
pg_upgrade so I doubt we are going to do this.
We cannot remove the check, or Nathan will come after us as he's
working hard on reducing the time pg_upgrade takes. We should not
make it longer if there is no need to.
The scans may be quite long as well, actually, which could be a
bottleneck. Did you measure the runtime with a maximized (still
realistic) pool of files for these SLRUs in the upgrade time? For
upgrades, data would be the neck.
# equals SLRU_SEG_FILENAMES_CHANGE_CAT_VER in pg_upgrade.h
my $slru_seg_filenames_change_cat_ver = 202411121;
[...]
open my $fh, "+<", $pg_control_fname or die $!;
binmode($fh);
sysseek($fh, 12, 0);
my $binval = pack("L!", $slru_seg_filenames_change_cat_ver - 1);
syswrite($fh, $binval, 4);
close($fh);
Control file manipulation may be useful as a routine in Cluster.pm,
based on an offset in the file and a format to pack as argument? Note
that this also depends on the system endianness, see 039_end_of_wal.pl.
It's one of these things I could see myself reuse to force a state in
the cluster and make a test cheaper, for example. You don't really
need the lookup part, actually? You would just need the part where
the control file is rewritten, which should be OK as long as the
cluster is freshly initdb'd meaning that there should be nothing that
interacts with the new value set. pg_upgrade only has CAT_VER flags
for some multixact changes and the jsonb check from 9.4.
--
Michael
Hi Michael,
The scans may be quite long as well, actually, which could be a
bottleneck. Did you measure the runtime with a maximized (still
realistic) pool of files for these SLRUs in the upgrade time? For
upgrades, data would be the neck.
Good question.
In theory SLRUs are not supposed to grow large and their size is a
small fraction of the rest of the database. As an example CLOG (
pg_xact/ ) stores 2 bits per transaction. Since every SLRU has a
dedicated directory and we scan just it, non-SLRU files don't affect
the scan time.
To make sure I asked several people to check how many SLRUs they have
in the prod environment. The typical response looked like this:
```
$PGDATA/pg_xact: 191 segments
$PGDATA/pg_commit_ts: 3
$PGDATA/pg_multixact/offsets: 148
$PGDATA/pg_multixact/members: 400
$PGDATA/pg_subtrans: 4
$PGDATA/pg_serial: 3
```
This is a 800 Gb database. Interestingly larger databases (4.2Tb) may
have much less SLRU segments (220 in total, most of them are pg_xact).
And here is the *worst* case that was reported to me:
```
$PGDATA/pg_xact: 171 segments
$PGDATA/pg_commit_ts: 3
$PGDATA/pg_multixact/offsets: 4864
$PGDATA/pg_multixact/members: 40996
$PGDATA/pg_subtrans: 5
$PGDATA/pg_serial: 3
```
I was told this is a "1Tb+" database. For this user pg_upgrade will
rename 45 000 files. I wrote a little script to check how much time it
will take:
```
#!/usr/bin/env perl
use strict;
my $from = "test_0001.tmp";
my $to = "test_0002.tmp";
system("touch $from");
for my $i (1..45000) {
rename($from, $to);
($from, $to) = ($to, $from);
}
```
On my laptop I get 0.5 seconds. Note that I don't do scanning, only
renaming, assuming that the recent should take most of the time. I
think this should be multiplied by 10 to take into account the role of
the filesystem cache and other factors.
All in all in the absolutely worst case scenario this shouldn't take
more than 5 seconds, in reality it will probably be orders of
magnitude less.
Note that this also depends on the system endianness, see 039_end_of_wal.pl.
Sure, I think I took it into account when using pack("L!"). My
understanding is that "L" takes care of the endiness since I see
special flags to force little- or big-endiness independently from the
platform [1]https://perldoc.perl.org/functions/pack. This of course should be tested in practice on different
machines. Using an exclamation mark in "L!" was a mistake since
cat_ver is not an int, but rather an uint32.
You don't really need the lookup part, actually?
For lookup we already have the pg_controldata tool, that's not a problem.
Control file manipulation may be useful as a routine in Cluster.pm,
based on an offset in the file and a format to pack as argument?
[...]
It's one of these things I could see myself reuse to force a state in
the cluster and make a test cheaper, for example.
You would just need the part where
the control file is rewritten, which should be OK as long as the
cluster is freshly initdb'd meaning that there should be nothing that
interacts with the new value set.
Agree. Still I don't see a good way of figuring out
sizeof(ControlFileData) from Perl. The structure has int's in it (e.g.
wal_level, MaxConnections, etc) thus the size is platform-dependent.
The CRC should be placed at the end of the structure. If we want to
manipulate MaxConnections etc their offsets are going to be
platform-dependent as well. And my understanding is that the alignment
is platform/compiler dependent too.
I guess we are going to need either a `pg_writecontoldata` tool or
`pg_controldata -w` flag. I wonder which option you find more
attractive, or maybe you have better ideas?
[1]: https://perldoc.perl.org/functions/pack
--
Best regards,
Aleksander Alekseev
Hi,
I guess we are going to need either a `pg_writecontoldata` tool or
`pg_controldata -w` flag. I wonder which option you find more
attractive, or maybe you have better ideas?
For the record, Michael and I had a brief discussion about this
offlist and decided to abandon the idea of adding TAP tests, relying
only on buildfarm. Also I will check if we have a clear error message
in case when a user forgot to run pg_upgrade and running new slru.c
with old filenames. If the user doesn't get such an error message I
will see if it's possible to add it somewhere in slru.c without
introducing much performance overhead.
Also I'm going to submit precise steps to test this migration manually
for the reviewers convenience.
--
Best regards,
Aleksander Alekseev
Hi,
For the record, Michael and I had a brief discussion about this
offlist and decided to abandon the idea of adding TAP tests, relying
only on buildfarm. Also I will check if we have a clear error message
in case when a user forgot to run pg_upgrade and running new slru.c
with old filenames. If the user doesn't get such an error message I
will see if it's possible to add it somewhere in slru.c without
introducing much performance overhead.Also I'm going to submit precise steps to test this migration manually
for the reviewers convenience.
Here is an updated patch. The steps to test it manually are as follows.
Compile and install PostgreSQL from the REL_17_STABLE branch:
```
git checkout REL_17_STABLE
git fetch origin
git rebase -i origin/REL_17_STABLE
git clean -dfx
meson setup --buildtype debug -Dcassert=true -Dtap_tests=enabled
-Dprefix=/home/eax/pginstall-17 build
ninja -C build
meson install -C build
~/pginstall-17/bin/initdb --data-checksums -D ~/pginstall-17/data
~/pginstall-17/bin/pg_ctl -D ~/pginstall-17/data -l
~/pginstall-17/data/logfile start
~/pginstall-17/bin/createdb $(whoami)
# fill DB (or even better - use a copy of an existing one), e.g:
~/pginstall-17/bin/pgbench -i -s 100
~/pginstall-17/bin/pgbench -j 16 -c 16 -T 10 -P 5
# should see 4-digit SLRU segment filenames, more files is better
ls -la ~/pginstall-17/data/pg_xact/ \
~/pginstall-17/data/pg_commit_ts/ \
~/pginstall-17/data/pg_multixact/members/ \
~/pginstall-17/data/pg_multixact/offsets/ \
~/pginstall-17/data/pg_subtrans/ \
~/pginstall-17/data/pg_serial/
~/pginstall-17/bin/pg_ctl -D ~/pginstall-17/data stop
```
Apply the patch to the `master` branch, recompile PostgreSQL, install
to the different location:
```
git checkout slru_pg_upgrade_v2
git clean -dfx
meson setup --buildtype debug -Dcassert=true -Dtap_tests=enabled
-Dprefix=/home/eax/pginstall-18 build
ninja -C build
meson install -C build
```
Try to start PostgreSQL without running pg_upgrade:
```
cp -r ~/pginstall-17/data ~/pginstall-18/data
~/pginstall-18/bin/pg_ctl -D ~/pginstall-18/data -l
~/pginstall-18/data/logfile start
```
You should get:
```
waiting for server to start.... stopped waiting
pg_ctl: could not start server
Examine the log output.
$ tail ~/pginstall-18/data/logfile
FATAL: database files are incompatible with server
DETAIL: The data directory was initialized by PostgreSQL version 17,
which is not compatible with this version 18devel
```
Run pg_upgrade:
```
rm -r ~/pginstall-18/data
~/pginstall-18/bin/initdb --data-checksums -D ~/pginstall-18/data
~/pginstall-18/bin/pg_upgrade
--old-datadir=/home/eax/pginstall-17/data
--new-datadir=/home/eax/pginstall-18/data
--old-bindir=/home/eax/pginstall-17/bin
--new-bindir=/home/eax/pginstall-18/bin
```
Make sure the output contains:
```
Renaming SLRU segments in pg_xact ok
Renaming SLRU segments in pg_commit_ts ok
Renaming SLRU segments in pg_multixact/offsets ok
Renaming SLRU segments in pg_multixact/members ok
Renaming SLRU segments in pg_subtrans ok
Renaming SLRU segments in pg_serial ok
```
Make sure PostgreSQL starts after the upgrade:
```
~/pginstall-18/bin/pg_ctl -D ~/pginstall-18/data -l
~/pginstall-18/data/logfile start
~/pginstall-18/bin/psql -c 'select count(*) from pgbench_accounts'
~/pginstall-18/bin/pg_ctl -D ~/pginstall-18/data stop
# should see 15-digit SLRU segment filenames
ls -la ~/pginstall-18/data/pg_xact/ \
~/pginstall-18/data/pg_commit_ts/ \
~/pginstall-18/data/pg_multixact/members/ \
~/pginstall-18/data/pg_multixact/offsets/ \
~/pginstall-18/data/pg_subtrans/ \
~/pginstall-18/data/pg_serial/
```
Make sure that the second run of pg_upgrade doesn't produce "Renaming
SLRU segments" messages:
```
mv ~/pginstall-18/data ~/pginstall-18/data.bak
~/pginstall-18/bin/initdb --data-checksums -D ~/pginstall-18/data
~/pginstall-18/bin/pg_upgrade
--old-datadir=/home/eax/pginstall-18/data.bak
--new-datadir=/home/eax/pginstall-18/data
--old-bindir=/home/eax/pginstall-18/bin
--new-bindir=/home/eax/pginstall-18/bin
```
As always, your feedback and suggestions are most welcomed.
--
Best regards,
Aleksander Alekseev
Attachments:
v2-0001-Always-use-long-SLRU-segment-file-names.patchapplication/octet-stream; name=v2-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From 7ae61fbb2fb310fafc5360222e28c881524ba83b Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v2] 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 0d556c00b8c..7a238efc227 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 95049acd0b5..99252cd9b87 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 27ccdf9500f..2cc64289054 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 7eeaafe2cb3..7e2a12d6a0d 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->bank_mask = (nslots / SLRU_BANK_SIZE) - 1;
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 36c7f3879d5..6d3dcc63d2b 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"
@@ -59,6 +60,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);
@@ -154,6 +157,7 @@ main(int argc, char **argv)
*/
copy_xact_xlog_xid();
+ check_slru_segment_filenames();
/* New now using xids of the old system */
@@ -806,6 +810,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 0cdd675e4f1..a839f19e310 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -125,6 +125,12 @@ extern char *output_files[];
*/
#define JSONB_FORMAT_CHANGE_CAT_VER 201409291
+/*
+ * 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 1111b09637d..9d1dbb93d72 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -237,7 +237,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 ae871b640f8..490ea85c5e3 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -133,13 +133,6 @@ typedef struct SlruCtlData
*/
bits16 bank_mask;
- /*
- * 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).
@@ -187,8 +180,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.47.1
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.
--
Best regards,
Aleksander Alekseev
Attachments:
v3-0001-Always-use-long-SLRU-segment-file-names.patchapplication/octet-stream; name=v3-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From 28e1dc9564256f2befe991dc9e89390f0f0f5c7c Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v3] 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 0d556c00b8c..7a238efc227 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 95049acd0b5..99252cd9b87 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 27ccdf9500f..2cc64289054 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 36c7f3879d5..6d3dcc63d2b 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"
@@ -59,6 +60,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);
@@ -154,6 +157,7 @@ main(int argc, char **argv)
*/
copy_xact_xlog_xid();
+ check_slru_segment_filenames();
/* New now using xids of the old system */
@@ -806,6 +810,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 0cdd675e4f1..a839f19e310 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -125,6 +125,12 @@ extern char *output_files[];
*/
#define JSONB_FORMAT_CHANGE_CAT_VER 201409291
+/*
+ * 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 1111b09637d..9d1dbb93d72 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -237,7 +237,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.47.1
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
Attachments:
v4-0001-Always-use-long-SLRU-segment-file-names.patchapplication/x-patch; name=v4-0001-Always-use-long-SLRU-segment-file-names.patchDownload
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
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
Hi Aleksander,
I have reviewed your patch. It looks good to me.
Kindest regards.
Rustam Allakov
The new status of this patch is: Ready for Committer
Hi Rustam,
Hi Aleksander,
I have reviewed your patch. It looks good to me.Kindest regards.
Rustam AllakovThe new status of this patch is: Ready for Committer
Thanks for testing!
Here is the rebased patch.
--
Best regards,
Aleksander Alekseev
Attachments:
v5-0001-Always-use-long-SLRU-segment-file-names.patchapplication/octet-stream; name=v5-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From b0b98310672de9181e7006fd28a6ed2e12e11851 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v5] 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, Rustam Allakov
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 | 71 ++++----------------
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, 103 insertions(+), 85 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 9d25a7df0d3..8d8f7d28a06 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 fe56286d9a9..752aa71fa87 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
*
* "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/%015" PRIX64, ctl->Dir, 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/%015" PRIX64, ctl->Dir, segno);
}
/*
@@ -250,7 +230,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;
@@ -341,7 +321,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));
}
@@ -1747,30 +1726,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.
*
@@ -1802,7 +1757,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 d82114ffca1..b029b12601e 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 9295e46aed3..df0bb7df1aa 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 */
@@ -843,6 +847,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 69c965bb7d0..32c6ac49138 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 84f23b8bc3d..df215378e74 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 2e4492900af..171076d9209 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.49.0
Hi,
The new status of this patch is: Ready for Committer
Thanks for testing!
Here is the rebased patch.
Rebased.
Attachments:
v6-0001-Always-use-long-SLRU-segment-file-names.patchtext/x-patch; charset=US-ASCII; name=v6-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From 4d83acd61eee36784840e2d1228d0c854807bac0 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v6] 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, Rustam Allakov
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 | 71 ++++----------------
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, 103 insertions(+), 85 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index e80fbe109cf..b6603cacf82 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -808,7 +808,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 370b38e048b..7798f217e75 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -554,8 +554,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 3cb09c3d598..d1c04948290 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1971,15 +1971,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 10ec259f382..3006def1f83 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
*
* "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/%015" PRIX64, ctl->Dir, 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/%015" PRIX64, ctl->Dir, segno);
}
/*
@@ -250,7 +230,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;
@@ -341,7 +321,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));
}
@@ -1772,30 +1751,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.
*
@@ -1827,7 +1782,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 09aace9e09f..517089f0666 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -242,7 +242,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 c07fb588355..d6117eb72f6 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 d5cd5bf0b3a..52f6a1cdfec 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
#include "postgres_fe.h"
+#include <dirent.h>
#include <time.h>
#include "catalog/pg_class_d.h"
@@ -63,6 +64,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);
@@ -161,6 +164,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 */
@@ -872,6 +876,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 0ef47be0dc1..73d102529c3 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 1dd60f709cf..deb8c11ca9e 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 20dbd1e0070..186d9cb1a10 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 void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 32750930e43..483c06edfc7 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.43.0
Hi,
The new status of this patch is: Ready for Committer
Thanks for testing!
Here is the rebased patch.
Rebased.
Rebased.
--
Best regards,
Aleksander Alekseev
Attachments:
v7-0001-Always-use-long-SLRU-segment-file-names.patchtext/x-patch; charset=US-ASCII; name=v7-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From 89ee21b95411af09aa5ec546cb6a59369653ff6d Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v7] 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, Rustam Allakov
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 | 71 ++++--------------
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 +--
.../test_slru/t/002_multixact_wraparound.pl | 6 +-
src/test/modules/test_slru/test_slru.c | 8 +-
13 files changed, 106 insertions(+), 88 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..e46dc13fb5a 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -809,7 +809,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 082b564da8f..25774fbc1b9 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -554,8 +554,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 3f423636b48..3482b1c16ad 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1736,15 +1736,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,
- true);
+ 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 549c7e3e64b..18baedd6de6 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
*
* "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/%015" PRIX64, ctl->Dir, 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/%015" PRIX64, ctl->Dir, 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));
}
@@ -1773,30 +1752,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.
*
@@ -1828,7 +1783,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 c0987f43f11..3536971ca7d 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -242,7 +242,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 40c42f572ed..0dd1a923fc9 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -536,7 +536,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 fe75ead3501..8e97bb35225 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 2127d297bfe..434843e4afe 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
#include "postgres_fe.h"
+#include <dirent.h>
#include <time.h>
#include "access/multixact.h"
@@ -64,6 +65,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);
@@ -162,6 +165,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 */
@@ -902,6 +906,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 ec018e4f292..58102e6a483 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -138,6 +138,12 @@ extern char *output_files[];
*/
#define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
+/*
+ * change of SLRU segment filenames length in PG19
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202601061
+
/*
* 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 b1d65b8aa0f..4677e9584ca 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 4cb8f478fce..541febf86bc 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -116,13 +116,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).
@@ -169,8 +162,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 void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
diff --git a/src/test/modules/test_slru/t/002_multixact_wraparound.pl b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
index 3793ac1c450..8b4601ab8e1 100644
--- a/src/test/modules/test_slru/t/002_multixact_wraparound.pl
+++ b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
@@ -40,7 +40,7 @@ my $slru_pages_per_segment = $1;
my $multixact_offsets_per_page = $blcksz / 8; # sizeof(MultiXactOffset) == 8
my $segno =
int(0xFFFFFFF8 / $multixact_offsets_per_page / $slru_pages_per_segment);
-my $slru_file = sprintf('%s/pg_multixact/offsets/%04X', $node_pgdata, $segno);
+my $slru_file = sprintf('%s/pg_multixact/offsets/%015X', $node_pgdata, $segno);
open my $fh, ">", $slru_file
or die "could not open \"$slru_file\": $!";
binmode $fh;
@@ -50,8 +50,8 @@ syswrite($fh, "\0" x $bytes_per_seg) == $bytes_per_seg
close $fh;
# remove old file
-unlink("$node_pgdata/pg_multixact/offsets/0000")
- or die "could not unlink \"$node_pgdata/pg_multixact/offsets/0000\": $!";
+unlink("$node_pgdata/pg_multixact/offsets/000000000000000")
+ or die "could not unlink \"$node_pgdata/pg_multixact/offsets/000000000000000\": $!";
# Consume multixids to wrap around. We start at 0xFFFFFFF8, so after
# creating 16 multixacts we should definitely have wrapped around.
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..642f1aeb8ca 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 = -1;
int test_buffer_tranche_id = -1;
@@ -247,8 +242,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.43.0
On 06/01/2026 17:18, Aleksander Alekseev wrote:
+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]); +}
Since commit bd8d9c9bdf "Widen MultiXactOffset to 64 bits",
"pg_multixact/members" should not be in that list anymore.
Also, it seems misleading that a function called "check_*" doesn't
merely check for things, but renames files. Also, it seems silly to
first copy/link the files with the old short names, and rename them
later. Could we copy/link them with the new long names to begin with?
(No comment on whether this is a good idea in general or the rest of the
patch)
- Heikki
Hi Heikki,
Since commit bd8d9c9bdf "Widen MultiXactOffset to 64 bits",
"pg_multixact/members" should not be in that list anymore.
I missed this one. Fixed, thanks.
Also, it seems misleading that a function called "check_*" doesn't
merely check for things, but renames files.
Fair point. I renamed it to `ensure_long_slru_segment_filenames`.
Could we copy/link them with the new long names to begin with?
That's an interesting idea.
What I personally don't like about it is the fact that a single
migration will affect the logic of every run of pg_upgrade, even in
the far future, for instances that don't need this migration.
Previously I showed [1]/messages/by-id/CAJ7c6TPdDSnVR2ZGn-oirfvvFXZghS5PAXYwmtb7_nNps53eEg@mail.gmail.com that the entire migration takes little time
(note that we had to migrate pg_multixact/members back then). So I
don't think this optimization is a good idea in the long run, unless
we reach a consensus on the opposite.
[1]: /messages/by-id/CAJ7c6TPdDSnVR2ZGn-oirfvvFXZghS5PAXYwmtb7_nNps53eEg@mail.gmail.com
--
Best regards,
Aleksander Alekseev
Attachments:
v8-0001-Always-use-long-SLRU-segment-file-names.patchtext/x-patch; charset=US-ASCII; name=v8-0001-Always-use-long-SLRU-segment-file-names.patchDownload
From 8223661f9194a76d1d3e09a2fad4ef3cec8f909f Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v8] 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.
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Rustam ALLAKOV <rustamallakov@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
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 | 71 ++++--------------
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 | 73 +++++++++++++++++++
src/bin/pg_upgrade/pg_upgrade.h | 6 ++
src/bin/pg_verifybackup/t/003_corruption.pl | 2 +-
src/include/access/slru.h | 10 +--
.../test_slru/t/002_multixact_wraparound.pl | 6 +-
src/test/modules/test_slru/test_slru.c | 8 +-
13 files changed, 105 insertions(+), 88 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..e46dc13fb5a 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -809,7 +809,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 082b564da8f..25774fbc1b9 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -554,8 +554,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 3f423636b48..3482b1c16ad 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1736,15 +1736,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,
- true);
+ 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 549c7e3e64b..18baedd6de6 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
*
* "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/%015" PRIX64, ctl->Dir, 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/%015" PRIX64, ctl->Dir, 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));
}
@@ -1773,30 +1752,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.
*
@@ -1828,7 +1783,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 c0987f43f11..3536971ca7d 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -242,7 +242,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 40c42f572ed..0dd1a923fc9 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -536,7 +536,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 fe75ead3501..8e97bb35225 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 2127d297bfe..94964f32eae 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
#include "postgres_fe.h"
+#include <dirent.h>
#include <time.h>
#include "access/multixact.h"
@@ -64,6 +65,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 ensure_long_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);
@@ -162,6 +165,7 @@ main(int argc, char **argv)
copy_xact_xlog_xid();
set_new_cluster_char_signedness();
+ ensure_long_slru_segment_filenames();
/* New now using xids of the old system */
@@ -902,6 +906,75 @@ 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
+ensure_long_slru_segment_filenames(void)
+{
+ int i;
+ static const char* dirs[] = {
+ "pg_xact",
+ "pg_commit_ts",
+ "pg_multixact/offsets",
+ "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 ec018e4f292..58102e6a483 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -138,6 +138,12 @@ extern char *output_files[];
*/
#define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
+/*
+ * change of SLRU segment filenames length in PG19
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202601061
+
/*
* 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 b1d65b8aa0f..4677e9584ca 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 4cb8f478fce..541febf86bc 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -116,13 +116,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).
@@ -169,8 +162,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 void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
diff --git a/src/test/modules/test_slru/t/002_multixact_wraparound.pl b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
index 3793ac1c450..8b4601ab8e1 100644
--- a/src/test/modules/test_slru/t/002_multixact_wraparound.pl
+++ b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
@@ -40,7 +40,7 @@ my $slru_pages_per_segment = $1;
my $multixact_offsets_per_page = $blcksz / 8; # sizeof(MultiXactOffset) == 8
my $segno =
int(0xFFFFFFF8 / $multixact_offsets_per_page / $slru_pages_per_segment);
-my $slru_file = sprintf('%s/pg_multixact/offsets/%04X', $node_pgdata, $segno);
+my $slru_file = sprintf('%s/pg_multixact/offsets/%015X', $node_pgdata, $segno);
open my $fh, ">", $slru_file
or die "could not open \"$slru_file\": $!";
binmode $fh;
@@ -50,8 +50,8 @@ syswrite($fh, "\0" x $bytes_per_seg) == $bytes_per_seg
close $fh;
# remove old file
-unlink("$node_pgdata/pg_multixact/offsets/0000")
- or die "could not unlink \"$node_pgdata/pg_multixact/offsets/0000\": $!";
+unlink("$node_pgdata/pg_multixact/offsets/000000000000000")
+ or die "could not unlink \"$node_pgdata/pg_multixact/offsets/000000000000000\": $!";
# Consume multixids to wrap around. We start at 0xFFFFFFF8, so after
# creating 16 multixacts we should definitely have wrapped around.
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..642f1aeb8ca 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 = -1;
int test_buffer_tranche_id = -1;
@@ -247,8 +242,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.43.0