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
