Re: Table refer leak in logical replication - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Table refer leak in logical replication
Date
Msg-id YIZdVHcgz9Z9QvKt@paquier.xyz
Whole thread Raw
In response to Re: Table refer leak in logical replication  (Amit Langote <amitlangote09@gmail.com>)
Responses Re: Table refer leak in logical replication  (Amit Langote <amitlangote09@gmail.com>)
List pgsql-hackers
On Fri, Apr 23, 2021 at 09:38:01PM +0900, Amit Langote wrote:
> On Thu, Apr 22, 2021 at 1:45 PM Michael Paquier <michael@paquier.xyz> wrote:
>> On the other hand, the tests for partitions have much more value IMO,
>> but looking closely I think that we can do better:
>> - Create triggers on more relations of the partition tree,
>> particularly to also check when a trigger does not fire.
>
> It seems you're suggesting to adopt 003's trigger firing tests for
> partitions in 013, but would we gain much by that?

I was suggesting the opposite, aka apply the trigger design that you
are introducing in 013 within 003.  But that may not be necessary
either :)

>> - Use a more generic name for tab1_2_op_log and its function
>> log_tab1_2_op(), say subs{1,2}_log_trigger_activity.
>
> Sure, done.

At the end I have used something simpler, as of
sub{1,2}_trigger_activity and sub{1,2}_trigger_activity_func.

>> - Create some extra BEFORE triggers perhaps?
>
> Again, that seems separate from what we're trying to do here.  AIUI,
> our aim here is to expand coverage for after triggers, and not really
> that of the trigger functionality proper, because nothing seems broken
> about it, but that of the trigger target relation opening/closing.
> ISTM that's what you're talking about below...

Exactly.  My review of the worker code is make me feeling that
reworking this code could easily lead to some incorrect behavior, so
I'd rather be careful with a couple of extra triggers created across
the partition tree, down to the partitions on which the triggers are
fired actually.

>> similarly to what issues_sql_like() or connect_{fails,ok}() do
>> already, but that would mean increasing the log level and we don't do
>> that to ease the load of the nodes.
>
> ...sorry, I am not very familiar with our Perl testing infra.  Is
> there some script that already does something like this?  For example,
> checking in the logs generated by a "node" that no instance of a
> certain WARNING is logged?

See for example how we test for SQL patterns with the backend logs
using issues_sql_like(), or the more recent connect_ok() and
connect_fails().  This functions match regexps with the logs of the
backend for patterns.  I am not sure if that's worth the extra cycles
though.  I also feel that we may want a more centralized place as well
to check such things, with more matching patterns, like at the end of
one run on a set of log files?

So, after a set of edits related to the format of the SQL queries, the
object names and some indentation (including a perltidy run), I have
applied this patch to close the loop.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Parallel INSERT SELECT take 2
Next
From: Peter Smith
Date:
Subject: Re: logical replication empty transactions