Re: Bug in pg_restore with EventTrigger in parallel mode

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: fabriziomello(at)gmail(dot)com, Michael Paquier <michael(at)paquier(dot)xyz>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Bug in pg_restore with EventTrigger in parallel mode
Date: 2020-03-07 23:42:16
Message-ID: 8858.1583624536@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

vignesh C <vignesh21(at)gmail(dot)com> writes:
> I'm not sure if we can add a test for this, can you have a thought
> about this to check if we can add a test.

Yeah, I'm not quite sure if a test is worth the trouble or not.

We clearly do need to restore event triggers later than we do now, even
without considering parallel restore: they should not be able to prevent
us from executing other restore actions. This is just like the rule that
we don't restore DML triggers until after we've loaded data.

However, I think that the existing code is correct to restore event
triggers before matview refreshes, not after as this patch would have us
do. The basic idea for matview refresh is that it should happen in the
normal running state of the database. If an event trigger interferes with
that, it would've done so in normal running as well.

I'm also not terribly on board with loading more functionality onto the
RestorePass mechanism. That's a crock that should go away someday,
because it basically duplicates and overrides pg_dump's normal object
sorting mechanism. So we don't want it doing more than it absolutely
has to. But in this case, I don't see any reason why we can't just
restore event triggers and matviews in the same post-ACL restore pass.
In a serial restore, that will make the event triggers come first
because of the existing sort rules. In a parallel restore, it's possible
that they'd be intermixed, but that doesn't bother me. Again, if your
event triggers have side-effects on your matview refreshes, you're
going to have some issues anyway.

So that leads me to the attached, which renames the "RESTORE_PASS_REFRESH"
symbol for clarity, and updates the pg_dump_sort.c code and comments
to match what's really going on.

regards, tom lane

Attachment Content-Type Size
event-trigger-restore-v3.patch text/x-diff 5.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-03-07 23:45:44 Re: range_agg
Previous Message Bruce Momjian 2020-03-07 23:24:56 Re: What's difference among insert/write/flush lsn?