Re: pgsql: Use the terminology "WAL file" not "log file" more consistently. - Mailing list pgsql-committers

On Thu, Sep 15, 2022 at 7:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andrew Dunstan <andrew@dunslane.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