Hi Robert,

On 3/24/17 3:00 PM, Robert Haas wrote:
On Wed, Mar 22, 2017 at 6:05 PM, David Steele <da...@pgmasters.net> wrote:
Wait, really?  I thought you abandoned this approach because there's
then no principled way to handle WAL segments of less than the default

I did say that, but I thought I had hit on a compromise.

But, as I originally pointed out the hex characters in the filename are not
aligned correctly for > 8 bits (< 16MB segments) and using different
alignments just made it less consistent.

I don't think I understand what the compromise is.  Are you saying we
should have one rule for segments < 16MB and another rule for segments
16MB?  I think using two different rules for file naming depending
on the segment size will be a negative for both tool authors and
ordinary users.

Sorry for the confusion, I meant to say that if we want to keep LSNs in the filenames and not change alignment for the current default, then we would need to drop support for segment sizes < 16MB (more or less what I said below). Bad editing on my part.

It would be OK if we were willing to drop the 1,2,4,8 segment sizes because
then the alignment would make sense and not change the current 16MB

Well, that is true.  But the thing I'm trying to do here is to keep
this patch down to what actually needs to be changed in order to
accomplish the original purchase, not squeeze more and more changes
into it.

Attached is a patch to be applied on top of Beena's v8 patch that preserves LSNs in the file naming for all segment sizes. It's not quite complete because it doesn't modify the lower size limit everywhere, but I think it's enough so you can see what I'm getting at. This passes check-world and I've poked at it in other segment sizes as well manually.

Behavior for the current default of 16MB is unchanged, and all other sizes go through a logical progression.






I believe that maintaining an easy correspondence between LSN and filename is important. The cluster will not always be up to help with these calculations and tools that do the job may not be present or may have issues.

I'm happy to merge this with Beena's patch (and tidy my patch up) if this looks like an improvement to everyone.

diff --git a/src/include/access/xlog_internal.h 
index e3f616a..08d6494 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -93,10 +93,12 @@ typedef XLogLongPageHeaderData *XLogLongPageHeader;
 extern uint32 XLogSegSize;
 #define XLOG_SEG_SIZE XLogSegSize
-/* XLogSegSize can range from 1MB to 1GB */
-#define XLogSegMinSize 1024 * 1024
+/* XLogSegSize can range from 16MB to 1GB */
+#define XLogSegMinSize 1024 * 1024 * 16
 #define XLogSegMaxSize 1024 * 1024 * 1024
+#define XLogSegMinSizeBits 24
 /* Default number of min and max wal segments */
@@ -158,10 +160,14 @@ extern uint32 XLogSegSize;
 /* Length of XLog file name */
 #define XLOG_FNAME_LEN    24
+#define XLogSegNoLower(logSegNo) \
+       (((logSegNo) % XLogSegmentsPerXLogId) * \
+               (XLogSegSize >> XLogSegMinSizeBits))
 #define XLogFileName(fname, tli, logSegNo)     \
        snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli,               \
                         (uint32) ((logSegNo) / XLogSegmentsPerXLogId), \
-                        (uint32) ((logSegNo) % XLogSegmentsPerXLogId))
+                        (uint32) XLogSegNoLower(logSegNo))
 #define XLogFileNameById(fname, tli, log, seg) \
        snprintf(fname, MAXFNAMELEN, "%08X%08X%08X", tli, log, seg)
@@ -181,17 +187,18 @@ extern uint32 XLogSegSize;
         strcmp((fname) + XLOG_FNAME_LEN, ".partial") == 0)
 #define XLogFromFileName(fname, tli, logSegNo) \
-       do {                                                                    
-               uint32 log;                                                     
-               uint32 seg;                                                     
-               sscanf(fname, "%08X%08X%08X", tli, &log, &seg); \
-               *logSegNo = (uint64) log * XLogSegmentsPerXLogId + seg; \
+       do {                                                                    
+               uint32 log;                                                     
+               uint32 seg;                                                     
+               sscanf(fname, "%08X%08X%08X", tli, &log, &seg);         \
+               *logSegNo = (uint64) log * XLogSegmentsPerXLogId +      \
+                       (seg / (XLogSegSize >> XLogSegMinSizeBits)); \
        } while (0)
 #define XLogFilePath(path, tli, logSegNo)      \
        snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X", tli,                 
                         (uint32) ((logSegNo) / XLogSegmentsPerXLogId),         
-                        (uint32) ((logSegNo) % XLogSegmentsPerXLogId))
+                        (uint32) XLogSegNoLower(logSegNo))
 #define TLHistoryFileName(fname, tli)  \
        snprintf(fname, MAXFNAMELEN, "%08X.history", tli)
@@ -210,7 +217,7 @@ extern uint32 XLogSegSize;
 #define BackupHistoryFileName(fname, tli, logSegNo, offset) \
        snprintf(fname, MAXFNAMELEN, "%08X%08X%08X.%08X.backup", tli, \
                         (uint32) ((logSegNo) / XLogSegmentsPerXLogId),         
-                        (uint32) ((logSegNo) % XLogSegmentsPerXLogId), offset)
+                        (uint32) XLogSegNoLower(logSegNo), offset)
 #define IsBackupHistoryFileName(fname) \
        (strlen(fname) > XLOG_FNAME_LEN && \
@@ -220,7 +227,7 @@ extern uint32 XLogSegSize;
 #define BackupHistoryFilePath(path, tli, logSegNo, offset)     \
        snprintf(path, MAXPGPATH, XLOGDIR "/%08X%08X%08X.%08X.backup", tli, \
                         (uint32) ((logSegNo) / XLogSegmentsPerXLogId), \
-                        (uint32) ((logSegNo) % XLogSegmentsPerXLogId), offset)
+                        (uint32) XLogSegNoLower(logSegNo), offset)
  * Information logged when we detect a change in one of the parameters
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to