Date: Sat, 12 Jun 2021 09:47:55 +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: <YMQEO1EYRQX1cWGQ@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>
 <2136546.1623457890@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="J8JGK+iJY2nwbqvc"
Content-Disposition: inline
In-Reply-To: <2136546.1623457890@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/YMQEO1EYRQX1cWGQ%40paquier.xyz>
Precedence: bulk


--J8JGK+iJY2nwbqvc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 11, 2021 at 08:31:30PM -0400, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> 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.

Perhaps.

>> +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().
>=20
> This coding is also stolen verbatim from other test scripts.
> What is your concern about it exactly?

My main concern are future changes in this test suite that could cause
more queries to generate logs that overlap with this sequence of
tests, falsifying what this test is checking.

> 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.)

The default TAP configuration does not use log rotation, because it
causes complications in the test code as it makes necessary wait
phases to make sure that the logs have rotated.  That's also more
costly than just getting the log position, run the queries and
slurp()'ing the file from the previous position.
--
Michael

--J8JGK+iJY2nwbqvc
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmDEBDsACgkQnvQgOdby
QH1/vBAAklTmOUdJAp6XnmIl0038Z9qgTecLbKhEmzcLfZYhEyZntsdFBIy/Sc+p
n5soO1I8cT9M1OVAFiC2dT+t/LaLua0gQG2ZM4sTuM4vhggouiY6KhyEhFWb4sk0
l51qh8wOw1/cGAYBFhNmMylJlrTzIyIVxGmP6Rvg8kPJD4yAwUN3zBc1ZoAwybLJ
h71x6qDmSvyz3a8/9Wf8JcIYYq5dMsnVIgegJ6Pwd3dTKLqW8g6b/igrEqXgwRTg
3PSi/yxv7uT/V/wzUF8kF4h1eZhk41A86eSHM4tbzhva+ip36kwojFGKJ1fc7NGZ
LIY6ezJGGeMvFO2XToaYdI9dxzi9YkBAt3RSIx/ybpys0tpMOXh7MLudICEzIahI
2Z/ZpQbTeOydo59WL9Dqh2H7DcswJKMr7J9HWsDy/0PzxdPKNi8vAb325u+Np3ab
rMNTxTPF4c8fhi69LtR+DEzM6xKexOoTdkuaqeHIrN4AmL6H5JuYPENBdMvL/dVf
UsHNMLnFLRTqIauxrPwQZo6Umaudvxf8KiYvlbUta5DbkLAuMM8vUpXJ98/+JUPe
xMf4mdEw5SAbqAujSRDxrQbdje7jlKLofMgomniO+Dq6hRxfb7uetMrZlw9wX4k9
i0j/zsl/YIrFScAgfMmQTB3m6MA4WVTDyLBw48oyxfp4uoiewmg=
=ePIZ
-----END PGP SIGNATURE-----

--J8JGK+iJY2nwbqvc--