Re: Race condition in recovery?

On Sat, Jun 12, 2021 at 10:20 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> > I have pushed a fix, tested on a replica of fairywren/drongo,
>
> This bit seems a bit random:
>
> # WAL segment, this is enough to guarantee that the history file was
> # archived.
> my $archive_wait_query =
> - "SELECT '$walfile_to_be_archived' <= last_archived_wal FROM pg_stat_archiver;";
> + "SELECT coalesce('$walfile_to_be_archived' <= last_archived_wal, false) " .
> + "FROM pg_stat_archiver";
> $node_standby->poll_query_until('postgres', $archive_wait_query)
> or die "Timed out while waiting for WAL segment to be archived";
> my $last_archived_wal_file = $walfile_to_be_archived;
>
> I wonder whether that is a workaround for the poll_query_until bug
> I proposed to fix at [1].

I found that a bit random too, but it wasn't the only part of the
patch I found a bit random. Like, what can this possibly be doing?

+if ($^O eq 'msys')
+{
+ $perlbin = TestLib::perl2host(dirname($^X)) . '\\' . basename($^X);
+}

The idea here is apparently that on msys, the directory name that is
part of $^X needs to be passed through perl2host, but the file name
that is part of the same $^X needs to NOT be passed through perl2host.
Is $^X really that broken? If so, I think some comments are in order.

+local $ENV{PERL_BADLANG}=0;

Similarly here. There's not a single other reference to PERL_BADLANG
in the repository, so if we need this one here, there should be a
comment explaining why this is different from all the places where we
don't need it.

On those occasions when I commit TAP test cases, I do try to think
about whether they are going to be portable, but I find these kinds of
changes indistinguishable from magic.

--
Robert Haas
EDB: http://www.enterprisedb.com