Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE
shveta malik <shveta.malik@gmail.com>
From: shveta malik <shveta.malik@gmail.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
shveta malik <shveta.malik@gmail.com>
Date: 2026-04-28T03:59:39Z
Lists: pgsql-hackers
On Tue, Apr 28, 2026 at 7:11 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Hi,
>
> On Mon, Apr 27, 2026 at 3:01 AM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > I've 2 comments:
>
> Thank you for reviewing!
>
> > 1/ What about having just one curr_idx increment? (right after list_nth(),
> > before the skip checks). I think that would be less error-prone if new skip
> > conditions are added later.
>
> Done.
>
> > 2/ I think that the test is racy and could also succeed even without the fixes.
> > Indeed, I think that the drops can complete before any concurrent polling
> > happens (I can see it by adding a pg_sleep(2) before the first poll in the DO
> > block). What about using an injection point to ensure a relation is removed
> > during the polling?
>
> I initially considered an injection point but chose polling since the
> TAP test reproduced the bug consistently with hundreds of tables on my
> dev system. I've now added an injection point for predictability.
>
> I adjusted the commit message a bit. Please find the attached v3 patch
> for further review. Thank you!
>
Thanks Bharath. I have just one minor comment:
+ INJECTION_POINT("pg-get-publication-tables-build-list", NULL);
Shall we name it as 'pg-get-publication-tables-list-built' to be more
meaningful, as we are pausing after list is built.
thanks
Shveta