From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Amit Langote <amitlangote09@gmail.com>, sbernikov@gmail.com,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17055: Logical replication worker crashes when applying
 update of row that dose not exist in target partiti
In-reply-to: <YMP+Gx2S8meYYHW4@paquier.xyz>
References: <17055-9ba800ec8522668b@postgresql.org>
 <1828113.1623370641@sss.pgh.pa.us>
 <CA+HiwqG05LNWKv=fJDtF93LFmA99KCSZtnAJUAsjBOLFOPy-fA@mail.gmail.com>
 <YMMCFPuQwO6sEcJ+@paquier.xyz>
 <CA+HiwqF0xi2KmPYKPPCtcgQ2vnjQrfm5cdY50VRC+S_s1M4KvQ@mail.gmail.com>
 <1881335.1623423667@sss.pgh.pa.us> <2058966.1623442520@sss.pgh.pa.us>
 <YMP+Gx2S8meYYHW4@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Sat, 12 Jun 2021 09:21:47 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2136545.1623457890.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 11 Jun 2021 20:31:30 -0400
Message-ID: <2136546.1623457890@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2136546.1623457890%40sss.pgh.pa.us>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> +$node_subscriber->append_conf('postgresql.conf', "log_min_messages =3D =
debug1");
> +$node_subscriber->reload;
> Wouldn't it be better to make sure that the change is effective before
> moving on to the end of the test?

AFAICS, none of the other callers of sub reload are doing anything
special to wait for it to take effect.  Not sure why this one
would need more protection.  If we do need a fix for that, likely
sub reload itself had better do it.

> +my $logfile =3D slurp_file($node_subscriber->logfile());
> +ok( $logfile =3D~
> +     qr/logical replication did not find row to be updated in replicati=
on target relation "tab_full_pk"/,
> +   'update target row is missing');
> I think that it would be better here to grab the log location before
> running the queries that will be matched with the logs, then pass down
> the location to slurp_file().

This coding is also stolen verbatim from other test scripts.
What is your concern about it exactly?  It's not like the
tests have such varying amounts of log output that an unexpected
log rotation would be likely.  (Does the default TAP setup even
allow log rotation at all?  I doubt it.)

			regards, tom lane