Re: pgsql: Use the terminology "WAL file" not "log file" more consistently.

On Thu, Sep 15, 2022 at 7:08 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > The pg_upgrade part of this seems seriously awry (see buildfarm animal
> > crake). We can't assume that the upgrade source is using the new wording.
>
> Ugh. I'll look into a fix tomorrow --- but my first instinct is
> that pg_upgrade shouldn't be so dependent on the exact wording of
> pg_controldata output.

Yeah, buildfarm member crake complains [1] about the change [2].
pg_upgrade basically expects that the controldata output be consistent
across source and target servers, the way it does is by invoking
pg_resetwal with -n option. Backporting [2] down to all the supported
PG versions doesn't work, because the pg_upgrade can fail while
upgrading from non-supported PG version without [2] to supported PG
version with [2].

We seem to be maintaining old code in pg_upgrade to not break it, see
[3]. I can think of a fix [4] or just [5], it may not be great to do
these kinds of fixes for all the pg_resetwal output changes. Since the
changes we make to the output formats of controlfile or pg_resetwal
are very rare, any of [4] or [5] looks good to me and it is better
than backporting [2] IMO.

Thoughts?

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-09-14%2023%3A17%3A32
[2]
index d4772a2965..7adf79eeed 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -788,7 +788,7 @@ PrintNewControlValues(void)

XLogFileName(fname, ControlFile.checkPointCopy.ThisTimeLineID,
newXlogSegNo, WalSegSz);
- printf(_("First log segment after reset: %s\n"), fname);
+ printf(_("First WAL segment after reset: %s\n"), fname);

[3]
else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)
else if ((p = strstr(bufin, "First log file segment after
reset:")) != NULL)

[4]
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 88d7e1c73d..c35eb84c20 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -348,7 +348,10 @@ get_control_data(ClusterInfo *cluster, bool live_check)
cluster->controldata.chkpnt_nxtmxoff = str2uint(p);
got_mxoff = true;
}
- else if ((p = strstr(bufin, "First WAL segment after
reset:")) != NULL)
+ else if ((GET_MAJOR_VERSION(cluster->major_version) < 1600 &&
+ (p = strstr(bufin, "First log
segment after reset:")) != NULL) ||
+
(GET_MAJOR_VERSION(cluster->major_version) >= 1600 &&
+ (p = strstr(bufin, "First WAL segment
after reset:")) != NULL))
{
/* Skip the colon and any whitespace after it */
p = strchr(p, ':');

[5]
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 88d7e1c73d..5386139b12 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -348,7 +348,8 @@ get_control_data(ClusterInfo *cluster, bool live_check)
cluster->controldata.chkpnt_nxtmxoff = str2uint(p);
got_mxoff = true;
}
- else if ((p = strstr(bufin, "First WAL segment after
reset:")) != NULL)
+ else if ((p = strstr(bufin, "First WAL segment after
reset:")) != NULL ||
+ (p = strstr(bufin, "First log segment
after reset:")) != NULL)
{
/* Skip the colon and any whitespace after it */
p = strchr(p, ':');

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com