From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: 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
Date: Sat, 12 Jun 2021 09:47:55 +0900
Message-ID: <YMQEO1EYRQX1cWGQ@paquier.xyz> (raw)
In-Reply-To: <2136546.1623457890@sss.pgh.pa.us>
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>
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 = slurp_file($node_subscriber->logfile());
>> +ok( $logfile =~
>> + qr/logical replication did not find row to be updated in replication 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?
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, tgl@sss.pgh.pa.us, amitlangote09@gmail.com, 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: <YMQEO1EYRQX1cWGQ@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox