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