On 3/21/18, 12:57 PM, "Peter Eisentraut" <peter.eisentraut@2ndquadrant.com> wrote:
> On 3/13/18 20:53, Bossart, Nathan wrote:
>> 0001: Fix division-by-zero error in pg_controldata
>
> committed that
Thanks!
>> 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?
With my patch, I was trying to still use the rest of the values in the
control file, altering only the WAL segment size. Also, I was doing a
bit of setup for 0003, where WalSegSz is used as the "new" WAL segment
size.
However, I think your approach is better. In addition to being much
simpler, it might make more sense to treat the control file as
corrupted if the WAL segment size is invalid, anyway. Any setup for
0003 can be easily moved, too.
Nathan
