Re: [HACKERS] pg_upgrade to clusters with a different WAL segment size

On 3/13/18 20:53, Bossart, Nathan wrote:
> Here is a new set of patches that addresses most of Peter's feedback.
> I've split it into four pieces:
> 
>   0001: Fix division-by-zero error in pg_controldata

committed that

>   0002: Fix division-by-zero error in pg_resetwal

This looks a bit more complicated than necessary to me.  I think there
is a mistake in the original patch fc49e24fa69:  In ReadControlFile(),
it says

/* return false if WalSegSz is not valid */

but then it doesn't actually do that.

If we make that change, then a wrong WAL segment size in the control
file would send us to GuessControlValues().  There, we need to set
WalSegSz, and everything would work.

diff --git a/src/bin/pg_resetwal/pg_resetwal.c
b/src/bin/pg_resetwal/pg_resetwal.c
index a132cf2e9f..c99e7a8db1 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -601,7 +601,7 @@ ReadControlFile(void)
            fprintf(stderr,
                    _("%s: pg_control specifies invalid WAL segment size
(%d bytes); proceed with caution \n"),
                    progname, WalSegSz);
-           guessed = true;
+           return false;
        }

        return true;
@@ -678,7 +678,7 @@ GuessControlValues(void)
    ControlFile.floatFormat = FLOATFORMAT_VALUE;
    ControlFile.blcksz = BLCKSZ;
    ControlFile.relseg_size = RELSEG_SIZE;
-   ControlFile.xlog_blcksz = XLOG_BLCKSZ;
+   WalSegSz = ControlFile.xlog_blcksz = XLOG_BLCKSZ;
    ControlFile.xlog_seg_size = DEFAULT_XLOG_SEG_SIZE;
    ControlFile.nameDataLen = NAMEDATALEN;
    ControlFile.indexMaxKeys = INDEX_MAX_KEYS;

What do you think?

Does your patch aim to do something different?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services