Hi,
> A comment about v3-0001.
>
> - * If true, use long segment filenames formed from lower 48 bits of
> the
> - * segment number, e.g. pg_xact/000000001234. Otherwise, use short
> - * filenames formed from lower 16 bits of the segment number e.g.
> - * pg_xact/1234.
> + * If true, use long segment filenames. Use short filenames otherwise.
> + * See SlruFileName().
>
> We're losing some details here even if SlruFileName() has some
> explanations, because one would need to read through the snprintf's
> 04X to know that short file names include 4 characters. I'm OK to
> mention SlruFileName() rather than duplicate the knowledge here, but
> SlruFileName() should also be updated to mention the same level of
> details with some examples of file names.
Fair enough. Here is the updated patchset.
--
Best regards,
Aleksander Alekseev
From 0b8d67ab6cc582b356e639175f85d30982f2fb2c Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 26 Jun 2024 12:59:15 +0300
Subject: [PATCH v4 1/2] Refactor the comments about formatting SLRU segment
filenames
The comment for SlruCtlData.long_segment_names was just wrong, an oversight
of commit 4ed8f0913bfd.
While on it, additionally clarify what SlruFileName() does and how it uses
SlruCtlData.long_segment_names.
Aleksander Alekseev. Reported by Noah Misch. Reviewed by Michael Paquier.
---
src/backend/access/transam/slru.c | 22 ++++++++++++++++++++++
src/include/access/slru.h | 15 +++++++++++----
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 77b05cc0a7..168df26065 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -72,6 +72,28 @@
#include "storage/shmem.h"
#include "utils/guc_hooks.h"
+/*
+ * Converts segment number to the filename of the segment.
+ *
+ * path: should point to a buffer at least MAXPGPATH characters long.
+ *
+ * If ctl->long_segment_names is true, segno can be in 0 .. (2^60-1) range and
+ * the resulting filename will be like:
+ *
+ * slru_dir/123456789ABCDEF (15 characters)
+ *
+ * This is the recommended way of formatting for all new code.
+ *
+ * If ctl->long_segment_names is false, segno can be in 0 .. (2^24-1) range.
+ * The resulting filename will be one of:
+ *
+ * slru_dir/1234 if 0 <= segno < 2^16
+ * slru_dir/12345 if 2^16 <= segno < 2^20
+ * slru_dir/123456 if 2^20 <= segno < 2^24
+ *
+ * This is the legacy way of formatting that is kept for backward compatibility.
+ * New code shouldn't use it.
+ */
static inline int
SlruFileName(SlruCtl ctl, char *path, int64 segno)
{
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 8a8d191873..c836fa8534 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -134,10 +134,17 @@ typedef struct SlruCtlData
bits16 bank_mask;
/*
- * If true, use long segment filenames formed from lower 48 bits of the
- * segment number, e.g. pg_xact/000000001234. Otherwise, use short
- * filenames formed from lower 16 bits of the segment number e.g.
- * pg_xact/1234.
+ * If true, use long segment filenames. Use short filenames otherwise.
+ *
+ * Each segment contains SLRU_PAGES_PER_SEGMENT pages, i.e.:
+ *
+ * Segment # Contains pages
+ * 0 0 .. (SLRU_PAGES_PER_SEGMENT-1)
+ * 1 SLRU_PAGES_PER_SEGMENT .. (SLRU_PAGES_PER_SEGMENT*2-1)
+ * ... etc ..
+ *
+ * The filename of the segment is formed from its number. For more details
+ * regarding exact rules see SlruFileName().
*/
bool long_segment_names;
--
2.45.2
From 6811fa964cce2f2f308793ca8d627d7c6dc3f1b2 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 26 Jun 2024 14:02:42 +0300
Subject: [PATCH v4 2/2] Use int64 for page numbers in clog.c & async.c
Oversight of 4ed8f0913bfd
Aleksander Alekseev, Noah Misch, Michael Paquier
---
src/backend/access/transam/clog.c | 4 ++--
src/backend/commands/async.c | 22 +++++++++++-----------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 44c253246b..e6f79320e9 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -445,7 +445,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
PGPROC *proc = MyProc;
uint32 nextidx;
uint32 wakeidx;
- int prevpageno;
+ int64 prevpageno;
LWLock *prevlock = NULL;
/* We should definitely have an XID whose status needs to be updated. */
@@ -577,7 +577,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
while (nextidx != INVALID_PROC_NUMBER)
{
PGPROC *nextproc = &ProcGlobal->allProcs[nextidx];
- int thispageno = nextproc->clogGroupMemberPage;
+ int64 thispageno = nextproc->clogGroupMemberPage;
/*
* If the page to update belongs to a different bank than the previous
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index ab4c72762d..3c43a700e7 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -283,7 +283,7 @@ typedef struct AsyncQueueControl
QueuePosition head; /* head points to the next free location */
QueuePosition tail; /* tail must be <= the queue position of every
* listening backend */
- int stopPage; /* oldest unrecycled page; must be <=
+ int64 stopPage; /* oldest unrecycled page; must be <=
* tail.page */
ProcNumber firstListener; /* id of first listener, or
* INVALID_PROC_NUMBER */
@@ -1271,9 +1271,9 @@ asyncQueueUnregister(void)
static bool
asyncQueueIsFull(void)
{
- int headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
- int tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
- int occupied = headPage - tailPage;
+ int64 headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
+ int64 tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
+ int64 occupied = headPage - tailPage;
return occupied >= max_notify_queue_pages;
}
@@ -1505,9 +1505,9 @@ pg_notification_queue_usage(PG_FUNCTION_ARGS)
static double
asyncQueueUsage(void)
{
- int headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
- int tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
- int occupied = headPage - tailPage;
+ int64 headPage = QUEUE_POS_PAGE(QUEUE_HEAD);
+ int64 tailPage = QUEUE_POS_PAGE(QUEUE_TAIL);
+ int64 occupied = headPage - tailPage;
if (occupied == 0)
return (double) 0; /* fast exit for common case */
@@ -1932,7 +1932,7 @@ asyncQueueReadAllNotifications(void)
do
{
- int curpage = QUEUE_POS_PAGE(pos);
+ int64 curpage = QUEUE_POS_PAGE(pos);
int curoffset = QUEUE_POS_OFFSET(pos);
int slotno;
int copysize;
@@ -2108,9 +2108,9 @@ static void
asyncQueueAdvanceTail(void)
{
QueuePosition min;
- int oldtailpage;
- int newtailpage;
- int boundary;
+ int64 oldtailpage;
+ int64 newtailpage;
+ int64 boundary;
/* Restrict task to one backend per cluster; see SimpleLruTruncate(). */
LWLockAcquire(NotifyQueueTailLock, LW_EXCLUSIVE);
--
2.45.2
