Re: [PoC] pg_upgrade: allow to upgrade publisher node

On Fri, Oct 27, 2023 at 2:26 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Thu, Oct 26, 2023 at 8:11 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> > The BF animal fairywren[1] failed when testing
> > 003_upgrade_logical_replication_slots.pl.
> >
> > From the log, I can see pg_upgrade failed to open the
> > invalid_logical_replication_slots.txt:
> >
> > # Checking for valid logical replication slots
> > # could not open file "C:/tools/nmsys64/home/pgrunner/bf/root/HEAD/pgsql.build/testrun/pg_upgrade/003_upgrade_logical_replication_slots/data/t_003_upgrade_logical_replication_slots_new_publisher_data/pgdata/pg_upgrade_output.d/20231026T112558.309/invalid_logical_replication_slots.txt": No such file or directory
> > # Failure, exiting
> >
> > The reason could be the length of this path(262) exceed the windows path
> > limit(260 IIRC). If so, I recall we fixed similar things before (e213de8e7) by
> > reducing the path somehow.
>
> Nice catch. Windows docs say that the file/directory path name can't
> exceed MAX_PATH, which is defined as 260 characters. However, one must
> opt-in to enable longer path names -
> https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry
> and https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry#enable-long-paths-in-windows-10-version-1607-and-later.
>
> > In this case, I think one approach is to reduce the file and testname to
> > xxx_logical_slots instead of xxx_logical_replication_slots. But we will analyze more
> > and share fix soon.
> >
> > [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&dt=2023-10-26%2009%3A04%3A54
>
> +1 for s/003_upgrade_logical_replication_slots.pl/003_upgrade_logical_slots.pl
> and s/invalid_logical_replication_slots.txt/invalid_logical_slots.txt.
> In fact, we've used "logical slots" instead of "logical replication
> slots" in the docs to be generic. By looking at the generated
> directory path name, I think we can use shorter node names - instead
> of old_publisher, new_publisher, subscriber - either use node1 (for
> old publisher), node2 (for subscriber), node3 (for new publisher) or
> use alpha (for old publisher), bravo (for subscriber), charlie (for
> new publisher) or such shorter names. We don't have to be that
> descriptive and long in node names, one can look at the test file to
> know which one is what.
>

Some more ideas for shortening the filename:

1. "003_upgrade_logical_replication_slots.pl" -- IMO the word
"upgrade" is redundant in that filename (earlier patches never had
this). The test file lives under "pg_upgrade/t" so I felt that
upgrading is already implied.

2. If the node names will be shortened they should still retain *some*
meaning if possible:
old_publisher/subscriber/new_publisher --> node1/node2/node3 (means
nothing without studying the tests)
old_publisher/subscriber/new_publisher --> alpha/bravo/charlie (means
nothing without studying the tests)
How about:
old_publisher/subscriber/new_publisher --> node_p1/node_s/node_p2
or similar...

======
Kind Regards,
Peter Smith.
Fujitsu Australia