RE: Fix 035_standby_logical_decoding.pl race conditions

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Bertrand Drouvot' <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Fix 035_standby_logical_decoding.pl race conditions
Date: 2025-04-01 01:22:49
Message-ID: OSCPR01MB14966351EDCB504D63A2D41D0F5AC2@OSCPR01MB14966.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Bertrand,

> s/to avoid the seeing a xl_running_xacts/to avoid seeing a xl_running_xacts/?

Fixed.

>
> === 2 (Nit)
>
> /* For testing slot invalidation due to the conflict */
>
> Not sure "due to the conflict" is needed.
>

OK, removed.

> ==== About PG17-v2-0001
>
> === 3
>
> The commit message still mentions injection point.

Oh, removed.

> === 4
>
> -# Note that pg_current_snapshot() is used to get the horizon. It does
> -# not generate a Transaction/COMMIT WAL record, decreasing the risk of
> -# seeing a xl_running_xacts that would advance an active replication slot's
> -# catalog_xmin. Advancing the active replication slot's catalog_xmin
> -# would break some tests that expect the active slot to conflict with
> -# the catalog xmin horizon.
>
> I'd be tempted to not remove this comment but reword it a bit instead. Something
> like?
>
> # Note that pg_current_snapshot() is used to get the horizon. It does
> # not generate a Transaction/COMMIT WAL record, decreasing the risk of
> # seeing a xl_running_xacts that would advance an active replication slot's
> # catalog_xmin. Advancing the active replication slot's catalog_xmin
> # would break some tests that expect the active slot to conflict with
> # the catalog xmin horizon. We ensure that active replication slots are not
> # created for tests that might produce this race condition though.

Added.

> === 6 (Nit)
>
> In drop_logical_slots(), s/needs_active_slot/drop_active_slot/?

Fixed.

> === 7 (Nit)
>
> In check_slots_conflict_reason(), s/needs_active_slot/checks_active_slot/?

Fixed.

> ==== About PG16-v2-0001
>
> Same as for PG17-v2-0001.

I followed all needed changes.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v3-0001-Stabilize-035_standby_logical_decoding.pl-by-usin.patch application/octet-stream 6.8 KB
PG16-v3-0001-Stabilize-035_standby_logical_decoding.pl-by-usin.patch application/octet-stream 15.0 KB
PG17-v3-0001-Stabilize-035_standby_logical_decoding.pl-by-usin.patch application/octet-stream 16.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tender Wang 2025-04-01 01:42:11 Re: [PATCH] Fix potential overflow in binary search mid calculation
Previous Message jian he 2025-04-01 01:01:10 Re: speedup COPY TO for partitioned table.