On Tue, Jun 10, 2025 at 11:55 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> Here is the V35 patch set which includes the following changes:
>
Thank You for the patches, few comments:
1)
compute_min_nonremovable_xid:
+ /*
+ * Stop advancing xmin if an invalid non-removable transaction ID is
+ * found, otherwise update xmin.
+ */
+ if (!TransactionIdIsValid(nonremovable_xid))
+ *can_advance_xmin = false;
IMO, we can not have invalid nonremovable_xid here. It may be a
possibility in a later patch where we do stop-conflict-detection for a
worker and resets nonremovable_xid to Invalid. But in patch003, should
this rather be an Assert as we want to ensure that worker's oldest is
set to slot's-xmin by this time.
2)
postgres=# create subscription sub1 connection 'dbname=postgres
host=localhost user=shveta port=5433' publication pub1 WITH
(retain_conflict_info = true);
NOTICE: deleted rows will continue to accumulate for detecting
conflicts until the subscription is enabled
NOTICE: created replication slot "sub1" on publisher
CREATE SUBSCRIPTION
postgres=# alter subscription sub2 enable;
NOTICE: deleted rows will continue to accumulate for detecting
conflicts until the subscription is enabled
We should not have this 'NOTICE: deleted rows' in above 2 commands.
3)
+
+ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
+DROP SUBSCRIPTION regress_testsub;
Shall we enable regress_testsub as well before we drop it to catch
unexpected notices/warnings if any?
4)
postgres=# alter subscription sub2 disable;
WARNING: commit timestamp and origin data required for detecting
conflicts won't be retained
HINT: Consider setting "track_commit_timestamp" to true.
Do we need "track_commit_timestamp" related WARNING while disabling
rci sub as well? I feel it should be there while enabling it alone.
thanks
Shveta
