Date: Sat, 12 Jun 2021 10:31:35 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
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
Message-ID: <YMQOd4RvCq1ui19U@paquier.xyz>
References: <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>
 <2136546.1623457890@sss.pgh.pa.us>
 <YMQEO1EYRQX1cWGQ@paquier.xyz>
 <2138662.1623460441@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="OoV2DzwlCzXQLMpc"
Content-Disposition: inline
In-Reply-To: <2138662.1623460441@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/YMQOd4RvCq1ui19U%40paquier.xyz>
Precedence: bulk


--OoV2DzwlCzXQLMpc
Content-Type: multipart/mixed; boundary="vMwr/Cicaa5vnq4S"
Content-Disposition: inline


--vMwr/Cicaa5vnq4S
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Jun 11, 2021 at 09:14:01PM -0400, Tom Lane wrote:
> Perhaps the buildfarm will prove me wrong, but I think we don't
> have to do anything.  And if we did want to do something, what
> would it be?  There is no feedback mechanism to let us observe
> when the worker processed the signal.

I am fine to do nothing here unless the buildfarm says something.

> Oh, I see, you wonder if there could already be entries like these
> in the file.  But since we only turned the log level up for the
> duration of the specific test, it doesn't seem like a problem.

That's not a problem, now.

> (I did think of this to the extent of making sure the second test
> group in 013_partition.pl looks for different messages than the
> first group does.)

Still, the current style is not a pattern I think we should spread
around, as a lot of copy-paste gets done in new tests added and one
would easily mix up queries with pattern checks.  The practice I'd
like to encourage is something like the attached, to make clear that
there is no risk to fetch incorrect logs.
--
Michael

--vMwr/Cicaa5vnq4S
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="subs-tap-logs.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscrip=
tion/t/001_rep_changes.pl
index b2fdb289db..abe5efd31d 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -309,6 +309,8 @@ is( $result, qq(1|bar
 $node_subscriber->append_conf('postgresql.conf', "log_min_messages =3D deb=
ug1");
 $node_subscriber->reload;
=20
+my $log_location =3D -s $node_subscriber->logfile;
+
 $node_subscriber->safe_psql('postgres', "DELETE FROM tab_full_pk");
=20
 $node_publisher->safe_psql('postgres',
@@ -317,7 +319,7 @@ $node_publisher->safe_psql('postgres', "DELETE FROM tab=
_full_pk WHERE a =3D 2");
=20
 $node_publisher->wait_for_catchup('tap_sub');
=20
-my $logfile =3D slurp_file($node_subscriber->logfile());
+my $logfile =3D slurp_file($node_subscriber->logfile, $log_location);
 ok( $logfile =3D~
 	  qr/logical replication did not find row to be updated in replication ta=
rget relation "tab_full_pk"/,
 	'update target row is missing');
diff --git a/src/test/subscription/t/013_partition.pl b/src/test/subscripti=
on/t/013_partition.pl
index d0b5a55d18..f309153cf9 100644
--- a/src/test/subscription/t/013_partition.pl
+++ b/src/test/subscription/t/013_partition.pl
@@ -354,6 +354,8 @@ $node_subscriber1->append_conf('postgresql.conf',
 	"log_min_messages =3D debug1");
 $node_subscriber1->reload;
=20
+my $log_location =3D -s $node_subscriber1->logfile;
+
 $node_publisher->safe_psql('postgres',
 	"INSERT INTO tab1 VALUES (1, 'foo'), (4, 'bar'), (10, 'baz')");
=20
@@ -369,7 +371,7 @@ $node_publisher->safe_psql('postgres', "DELETE FROM tab=
1");
 $node_publisher->wait_for_catchup('sub1');
 $node_publisher->wait_for_catchup('sub2');
=20
-my $logfile =3D slurp_file($node_subscriber1->logfile());
+my $logfile =3D slurp_file($node_subscriber1->logfile(), $log_location);
 ok( $logfile =3D~
 	  qr/logical replication did not find row to be updated in replication ta=
rget relation's partition "tab1_2_2"/,
 	'update target row is missing in tab1_2_2');

--vMwr/Cicaa5vnq4S--

--OoV2DzwlCzXQLMpc
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmDEDncACgkQnvQgOdby
QH1yBQ//XwLF49INEtPgOjWpeP8HOb4ke3bRQ8+/8+pzzVzhe5LN6iAGyqOYzkTa
yPHN9a1CywOc3GId2sTgkOU2hb+hTcRrKFT1zTceGpem/iuQfjRVdmHaFUyYM+ce
WA401AHzLK1UnYq+GOXR/3Qxql3lgm7gpqJnZkRGNvlAdBFBxaq83D4gfSohPy4A
/TJOOU+piLVKzvkWSxYZuDyDFfxTySob5yIBUvcJlOlDhM1x2eoU3U+kkEJFrJZH
dg3UqziNArLOK7M3HbA9WGbCz+O8DE+z3Laj7CdtyJBkGIESU+HtYrr/nX7s2vgx
ftBd+aK1MvXXG4inyozv+nL753+6QwbKvAyQmkYxFhHIXy2jwVVawx5GVZqvEl2N
ahTb7M75mt5qM5wz2A4UcakWSsxzOsfzUeu1ZJiER+YHugSo1U1S/1ujLPSr+bzy
eImedNFuF8K1ZzaCdPczjWH+jjW8lFpu9YaYGHb7W8E9UICzLv2J+/GKDtX2fOiH
+PVL5N4qDArOU+pEwjteB7WRlCVX5XUnGn2z5vseZbo5kdxNbndPSMi8DnHdiseM
XqgwbiftwGCwCKUD9X4p7Bf+4xqIIqYNpjU3F4LFKflctciR13HPswpHiJfkM4zI
DhXCtExPtEn5b6pgFIGZ+1/TnlOW3S0IOXxXRzrbNJvMnLQdfu8=
=/mAz
-----END PGP SIGNATURE-----

--OoV2DzwlCzXQLMpc--