| From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
|---|---|
| To: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
| Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Vitaly Davydov <v(dot)davydov(at)postgrespro(dot)ru>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "suyu(dot)cmj" <mengjuan(dot)cmj(at)alibaba-inc(dot)com>, tomas <tomas(at)vondra(dot)me>, michael <michael(at)paquier(dot)xyz>, "bharath(dot)rupireddyforpostgres" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
| Subject: | Re: Newly created replication slot may be invalidated by checkpoint |
| Date: | 2026-01-23 09:06:25 |
| Message-ID: | CAA4eK1+T8a7JysOcM6PL1ycfQ6yXvJdDkzrkGOBZGj=fo7S7Lw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Jan 23, 2026 at 7:33 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> This patch cannot be applied cleanly on backbranches, I can prepare patches for
> those once the main patch is stable.
>
Some comments:
1.
+ /*
+ * Determine the minimum non-removable LSN by comparing the redo pointer
+ * with the minimum slot LSN.
+ */
+ min_safe_lsn = GetRedoRecPtr();
+ slot_min_lsn = XLogGetReplicationSlotMinimumLSN();
Can we expand these comments a bit to state why we need both
RedoRecPtr and slot's minimum LSN?
2.
+# Verify that while syncing a slot to the standby server, if the WAL before the
+# remote restart_lsn is at risk of being removed by a checkpoint, the slot
+# cannot be synced. Otherwise, even if the slot syncing succeeds, it may be
+# immediately invalidated by the checkpoint.
+my $primary = $node;
This comment atop the testcase is not very clear. Because, it is
testing that the slot is synced and is not invalidated. How about:
"Verify that the synchronized slots won't be invalidated immediately
after synchronization in the presence of a concurrent checkpoint."?
3.
+# Increase the log_min_messages setting to DEBUG2 on both the standby and
+# primary to debug test failures, if any.
+my $connstr_1 = $primary->connstr;
Do we need this DEBUG2? I don't think we should add too many DEBUG2
tests as it increases Log volume.
--
With Regards,
Amit Kapila.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Eugeny Goryachev | 2026-01-23 09:41:03 | [PATCH] Avoid potential NULL dereference in LIKE/ILIKE with C locale |
| Previous Message | Xuneng Zhou | 2026-01-23 08:47:43 | Re: Add WALRCV_CONNECTING state to walreceiver |