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

Thanks for taking a look.

On 3/3/18, 12:22 PM, "Peter Eisentraut" <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 2/7/18 13:34, Bossart, Nathan wrote:
>> Here is a patch to allow users to change the WAL segment size of a cluster with
>> pg_resetwal. Like the '--wal-segize' option in initdb, the new '-s' option
>> accepts segment sizes in megabytes.
>
> Or how about we call the new option, um, --wal-segsize?

It doesn't look like pg_resetwal takes any long options except for
--help and --version, although I suppose those could be added as part
of this effort.

>> The first is a division-by-zero error caused when the control data must be
>> guessed. If the control data is damaged, WalSegSz will not be set to the
>> guessed WAL segment size, resulting in an error during XLogFromFileName(). The
>> attached patch resolves this issue by ensuring WalSegSz is set to a valid value
>> after either reading or guessing the control values.
>
> I noticed this issue in pg_controldata too. Maybe that should be
> combined in a separate patch.

IIUC you are talking about the following code in pg_controldata:

/*
* Calculate name of the WAL file containing the latest checkpoint's REDO
* start point.
*/
XLByteToSeg(ControlFile->checkPointCopy.redo, segno, WalSegSz);
XLogFileName(xlogfilename, ControlFile->checkPointCopy.ThisTimeLineID,
segno, WalSegSz);

If the control file says that the WAL segment size is 0 bytes, then
XLByteToSeg() will indeed fail. I think a similar issue is still
present for XLogFromFileName() in pg_resetwal even with this patch.
For pg_controldata, perhaps the affected output ("Latest checkpoint's
REDO WAL file") should be marked unknown if the control file specifies
a 0-byte WAL segment size. For pg_resetwal, perhaps the WAL segment
size should be "guessed" in this case.

> The patch "Configurable file mode mask" contains the beginning of a test
> suite for pg_resetwal. It would be great if we could get a test case
> for this new functionality implemented.

I agree. I will take a look at that patch set.

Nathan