On Wed, Sep 18, 2024 at 3:31 PM shveta malik wrote:
>
> On Wed, Sep 18, 2024 at 2:49 PM shveta malik wrote:
> >
> > > > Please find the attached v46 patch having changes for the above review
> > > > comments and your test review comments and Shveta's review comments.
> > > >
>
When we promote ho
On Mon, Sep 16, 2024 at 10:41 PM Bharath Rupireddy
wrote:
>
> Thanks for looking into this.
>
> On Mon, Sep 16, 2024 at 4:54 PM Amit Kapila wrote:
> >
> > Why raise the ERROR just for timeout invalidation here and why not if
> > the slot is invalidated for other reasons? This raises the question
On Wed, Sep 18, 2024 at 2:49 PM shveta malik wrote:
>
> > > Please find the attached v46 patch having changes for the above review
> > > comments and your test review comments and Shveta's review comments.
> > >
When the synced slot is marked as 'inactive_timeout' invalidated on
hot standby due t
On Wed, Sep 18, 2024 at 12:21 PM shveta malik wrote:
>
> On Mon, Sep 16, 2024 at 3:31 PM Bharath Rupireddy
> wrote:
> >
> > Hi,
> >
> >
> > Please find the attached v46 patch having changes for the above review
> > comments and your test review comments and Shveta's review comments.
> >
>
> Thank
On Mon, Sep 16, 2024 at 3:31 PM Bharath Rupireddy
wrote:
>
> Hi,
>
>
> Please find the attached v46 patch having changes for the above review
> comments and your test review comments and Shveta's review comments.
>
Thanks for addressing comments.
Is there a reason that we don't support this inva
Here are a few comments for the patch v46-0001.
==
src/backend/replication/slot.c
1. ReportSlotInvalidation
On Mon, Sep 16, 2024 at 8:01 PM Bharath Rupireddy
wrote:
>
> On Mon, Sep 9, 2024 at 1:11 PM Peter Smith wrote:
> > 3. ReportSlotInvalidation
> >
> > I didn't understand why there was
Hi,
Thanks for looking into this.
On Mon, Sep 16, 2024 at 4:54 PM Amit Kapila wrote:
>
> Why raise the ERROR just for timeout invalidation here and why not if
> the slot is invalidated for other reasons? This raises the question of
> what happens before this patch if the invalid slot is used fro
On Mon, Sep 16, 2024 at 3:31 PM Bharath Rupireddy
wrote:
>
> Please find the attached v46 patch having changes for the above review
> comments and your test review comments and Shveta's review comments.
>
-ReplicationSlotAcquire(const char *name, bool nowait)
+ReplicationSlotAcquire(const char *n
Hi,
Thanks for reviewing.
On Mon, Sep 9, 2024 at 1:11 PM Peter Smith wrote:
>
> 1.
> +Note that the inactive timeout invalidation mechanism is not
> +applicable for slots on the standby server that are being synced
> +from primary server (i.e., standby slots having
>
> ni
Hi,
Thanks for reviewing.
On Mon, Sep 9, 2024 at 10:54 AM shveta malik wrote:
>
> 2)
> src/sgml/config.sgml:
>
> + disables the inactive timeout invalidation mechanism
>
> + Slot invalidation due to inactivity timeout occurs during checkpoint.
>
> Either have 'inactive' at both the places or 'in
On Tue, Sep 10, 2024 at 12:13 AM Bharath Rupireddy
wrote:
>
> On Mon, Sep 9, 2024 at 3:04 PM Amit Kapila wrote:
> >
> > > > > > We should not allow the invalid replication slot to be altered
> > > > > > irrespective of the reason unless there is any benefit.
> > > > >
> > > > > Okay, then I think
Hi, here is the remainder of my v45-0001 review. These comments are
for the test code only.
==
Testcase #1
1.
+# Testcase start
+#
+# Invalidate streaming standby slot and logical failover slot on primary due to
+# inactive timeout. Also, check logical failover slot synced to standby from
+#
Hi,
On Mon, Sep 9, 2024 at 3:04 PM Amit Kapila wrote:
>
> > > > > We should not allow the invalid replication slot to be altered
> > > > > irrespective of the reason unless there is any benefit.
> > > >
> > > > Okay, then I think we need to change the existing behaviour of the
> > > > other inval
On Mon, Sep 9, 2024 at 10:28 AM shveta malik wrote:
>
> On Mon, Sep 9, 2024 at 10:26 AM Bharath Rupireddy
> wrote:
> >
> > Hi,
> >
> > On Mon, Sep 9, 2024 at 9:17 AM shveta malik wrote:
> > >
> > > > We should not allow the invalid replication slot to be altered
> > > > irrespective of the reaso
Hi, here are some review comments for v45-0001 (excluding the test code)
==
doc/src/sgml/config.sgml
1.
+Note that the inactive timeout invalidation mechanism is not
+applicable for slots on the standby server that are being synced
+from primary server (i.e., standby s
On Sun, Sep 8, 2024 at 5:25 PM Bharath Rupireddy
wrote:
>
>
> Please find the v45 patch. Addressed above and Shveta's review comments [1].
>
Thanks for the patch. Please find my comments:
1)
src/sgml/config.sgml:
+ Synced slots are always considered to be inactive because they
don't perform lo
On Mon, Sep 9, 2024 at 10:26 AM Bharath Rupireddy
wrote:
>
> Hi,
>
> On Mon, Sep 9, 2024 at 9:17 AM shveta malik wrote:
> >
> > > We should not allow the invalid replication slot to be altered
> > > irrespective of the reason unless there is any benefit.
> >
> > Okay, then I think we need to chan
Hi,
On Mon, Sep 9, 2024 at 9:17 AM shveta malik wrote:
>
> > We should not allow the invalid replication slot to be altered
> > irrespective of the reason unless there is any benefit.
>
> Okay, then I think we need to change the existing behaviour of the
> other invalidation causes which still al
On Thu, Sep 5, 2024 at 9:30 AM Amit Kapila wrote:
>
> On Wed, Sep 4, 2024 at 9:17 AM shveta malik wrote:
> >
> > On Tue, Sep 3, 2024 at 3:01 PM shveta malik wrote:
> > >
> > >
> > > 1)
> > > I see that ReplicationSlotAlter() will error out if the slot is
> > > invalidated due to timeout. I have
Hi,
Thanks for reviewing.
On Tue, Sep 3, 2024 at 3:01 PM shveta malik wrote:
>
> 1)
> I see that ReplicationSlotAlter() will error out if the slot is
> invalidated due to timeout. I have not tested it myself, but do you
> know if slot-alter errors out for other invalidation causes as well?
> Ju
Hi,
Thanks for reviewing.
On Mon, Sep 2, 2024 at 1:37 PM Peter Smith wrote:
>
> Commit message.
>
> 1.
> Because such synced slots are typically considered not
> active (for them to be later considered as inactive) as they don't
> perform logical decoding to produce the changes.
>
> This sentenc
On Wed, Sep 4, 2024 at 9:17 AM shveta malik wrote:
>
> On Tue, Sep 3, 2024 at 3:01 PM shveta malik wrote:
> >
> >
> > 1)
> > I see that ReplicationSlotAlter() will error out if the slot is
> > invalidated due to timeout. I have not tested it myself, but do you
> > know if slot-alter errors out f
On Wed, Sep 4, 2024 at 2:49 PM shveta malik wrote:
>
> On Wed, Sep 4, 2024 at 9:17 AM shveta malik wrote:
> >
> > On Tue, Sep 3, 2024 at 3:01 PM shveta malik wrote:
> > >
> > >
>
>
> 1)
> It is related to one of my previous comments (pt 3 in [1]) where I
> stated that inactive_since should not k
On Wed, Sep 4, 2024 at 9:17 AM shveta malik wrote:
>
> On Tue, Sep 3, 2024 at 3:01 PM shveta malik wrote:
> >
> >
1)
It is related to one of my previous comments (pt 3 in [1]) where I
stated that inactive_since should not keep on changing once a slot is
invalidated.
Below is one side effect if
On Tue, Sep 3, 2024 at 3:01 PM shveta malik wrote:
>
>
> 1)
> I see that ReplicationSlotAlter() will error out if the slot is
> invalidated due to timeout. I have not tested it myself, but do you
> know if slot-alter errors out for other invalidation causes as well?
> Just wanted to confirm that
On Sat, Aug 31, 2024 at 1:45 PM Bharath Rupireddy
wrote:
>
> Hi,
>
>
> Please find the attached v44 patch with the above changes. I will
> include the 0002 xid_age based invalidation patch later.
>
Thanks for the patch Bharath. My review and testing is WIP, but please
find few comments and querie
Hi, my previous review posts did not cover the test code.
Here are my review comments for the v44-0001 test code
==
TEST CASE #1
1.
+# Wait for the inactive replication slot to be invalidated.
+$standby1->poll_query_until(
+ 'postgres', qq[
+ SELECT COUNT(slot_name) = 1 FROM pg_replication_s
On Sat, Aug 31, 2024 at 1:45 PM Bharath Rupireddy
wrote:
>
> Please find the attached v44 patch with the above changes. I will
> include the 0002 xid_age based invalidation patch later.
>
It is better to get the 0001 reviewed and committed first. We can
discuss about 0002 afterwards as 0001 is in
Hi. Thanks for addressing my previous review comments.
Here are some review comments for v44-0001.
==
Commit message.
1.
Because such synced slots are typically considered not
active (for them to be later considered as inactive) as they don't
perform logical decoding to produce the changes.
On Thu, Aug 29, 2024 at 11:31 AM Bharath Rupireddy
wrote:
>
> Thanks for looking into this.
>
> On Mon, Aug 26, 2024 at 4:35 PM Amit Kapila wrote:
> >
> > Few comments on 0001:
> > 1.
> > @@ -651,6 +651,13 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid
> >
> > + /*
> > + * Skip the sync if
Hi,
Thanks for looking into this.
On Fri, Aug 30, 2024 at 8:13 AM Peter Smith wrote:
>
> ==
> Commit message
>
> 1.
> ... introduces a GUC allowing users set inactive timeout.
>
> ~
>
> 1a. You should give the name of the new GUC in the commit message.
Modified.
> 1b. /set/to set/
Reworde
Hi, here are some review comments for patch v43-0001.
==
Commit message
1.
... introduces a GUC allowing users set inactive timeout.
~
1a. You should give the name of the new GUC in the commit message.
1b. /set/to set/
==
doc/src/sgml/config.sgml
GUC "replication_slot_inactive_timeou
Hi,
Thanks for looking into this.
On Mon, Aug 26, 2024 at 4:35 PM Amit Kapila wrote:
>
> Few comments on 0001:
> 1.
> @@ -651,6 +651,13 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid
>
> + /*
> + * Skip the sync if the local slot is already invalidated. We do this
> + * beforehand to avoid
On Mon, Aug 26, 2024 at 11:44 AM Bharath Rupireddy
wrote:
>
Few comments on 0001:
1.
@@ -651,6 +651,13 @@ synchronize_one_slot(RemoteSlot *remote_slot, Oid
remote_dbid)
" name slot \"%s\" already exists on the standby",
remote_slot->name));
+ /*
+ * Skip the sync if the local slot is a
On Wed, Aug 14, 2024 at 9:20 AM Ajin Cherian wrote:
>
> Some minor comments on the patch:
Thanks for reviewing.
> 1.
> + /*
> + * Release the lock if it's not yet to keep the cleanup path on
> + * error happy.
> + */
>
> I suggest rephrasing to: " "Release the lock if it hasn't been already to
On Mon, Jun 24, 2024 at 4:01 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> Hi,
>
> On Mon, Jun 17, 2024 at 5:55 PM Bharath Rupireddy
> wrote:
>
> Please find the attached v41 patches implementing the idea of vacuum
> doing the invalidation.
>
> Thoughts?
>
>
>
Some mino
On Tue, Jul 9, 2024 at 3:01 PM Nathan Bossart wrote:
>
> On Mon, Jun 24, 2024 at 11:30:00AM +0530, Bharath Rupireddy wrote:
> > 6) Vacuum command can't be run on the standby in recovery. So, to help
> > invalidate replication slots on the standby, I have for now let the
> > checkpointer also do th
On Mon, Jun 24, 2024 at 4:01 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> Hi,
>
> On Mon, Jun 17, 2024 at 5:55 PM Bharath Rupireddy
> wrote:
> >
> > Here are my thoughts on when to do the XID age invalidation. In all
> > the patches sent so far, the XID age invalidation
On Mon, Jun 24, 2024 at 11:30:00AM +0530, Bharath Rupireddy wrote:
> 6) Vacuum command can't be run on the standby in recovery. So, to help
> invalidate replication slots on the standby, I have for now let the
> checkpointer also do the XID age based invalidation. I know
> invalidating both in chec
Hi,
On Mon, Jun 17, 2024 at 5:55 PM Bharath Rupireddy
wrote:
>
> Here are my thoughts on when to do the XID age invalidation. In all
> the patches sent so far, the XID age invalidation happens in two
> places - one during the slot acquisition, and another during the
> checkpoint. As the suggestio
On Mon, Jun 17, 2024 at 05:55:04PM +0530, Bharath Rupireddy wrote:
> Here are my thoughts on when to do the XID age invalidation. In all
> the patches sent so far, the XID age invalidation happens in two
> places - one during the slot acquisition, and another during the
> checkpoint. As the suggest
Hi,
On Sat, Apr 13, 2024 at 9:36 AM Bharath Rupireddy
wrote:
>
> There was a point raised by Amit
> https://www.postgresql.org/message-id/CAA4eK1K8wqLsMw6j0hE_SFoWAeo3Kw8UNnMfhsWaYDF1GWYQ%2Bg%40mail.gmail.com
> on when to do the XID age based invalidation - whether in checkpointer
> or when vacuu
On Thu, Apr 25, 2024 at 11:11 AM Bharath Rupireddy
wrote:
>
> On Mon, Apr 22, 2024 at 7:21 PM Masahiko Sawada wrote:
> >
> > > Please find the attached v35 patch.
> >
> > The documentation says about both 'active' and 'inactive_since'
> > columns of pg_replication_slots say:
> >
> > ---
> > activ
On Mon, Apr 22, 2024 at 7:21 PM Masahiko Sawada wrote:
>
> > Please find the attached v35 patch.
>
> The documentation says about both 'active' and 'inactive_since'
> columns of pg_replication_slots say:
>
> ---
> active bool
> True if this slot is currently actively being used
>
> inactive_since
Hi,
On Thu, Apr 4, 2024 at 9:23 PM Bharath Rupireddy
wrote:
>
> On Thu, Apr 4, 2024 at 4:35 PM Amit Kapila wrote:
> >
> > > Thanks for the changes. v34-0001 LGTM.
> >
> > I was doing a final review before pushing 0001 and found that
> > 'inactive_since' could be set twice during startup after pr
On Sat, Apr 6, 2024 at 5:10 PM Bharath Rupireddy
wrote:
>
> Please see the attached v38 patch.
Hi, thanks everyone for reviewing the design and patches so far. Here
I'm with the v39 patches implementing inactive timeout based (0001)
and XID age based (0002) invalidation mechanisms.
I'm quoting t
On Sat, Apr 6, 2024 at 12:18 PM Amit Kapila wrote:
>
> Why the handling w.r.t active_pid in InvalidatePossiblyInactiveSlot()
> is not similar to InvalidatePossiblyObsoleteSlot(). Won't we need to
> ensure that there is no other active slot user? Is it sufficient to
> check inactive_since for the s
On Sat, Apr 6, 2024 at 11:55 AM Bharath Rupireddy
wrote:
>
Why the handling w.r.t active_pid in InvalidatePossiblyInactiveSlot()
is not similar to InvalidatePossiblyObsoleteSlot(). Won't we need to
ensure that there is no other active slot user? Is it sufficient to
check inactive_since for the sa
On Fri, Apr 5, 2024 at 1:14 PM Bertrand Drouvot
wrote:
>
> > Please find the attached v36 patch.
>
> A few comments:
>
> 1 ===
>
> +
> +The timeout is measured from the time since the slot has become
> +inactive (known from its
> +inactive_since value) until it gets
Hi,
On Fri, Apr 05, 2024 at 11:21:43AM +0530, Bharath Rupireddy wrote:
> On Wed, Apr 3, 2024 at 9:57 PM Bertrand Drouvot
> wrote:
> Please find the attached v36 patch.
Thanks!
A few comments:
1 ===
+
+The timeout is measured from the time since the slot has become
+inac
On Wed, Apr 3, 2024 at 9:57 PM Bertrand Drouvot
wrote:
>
> > > shouldn't the slot be dropped/recreated instead of updating
> > > inactive_since?
> >
> > The sync slots that are invalidated on the primary aren't dropped and
> > recreated on the standby.
>
> Yeah, right (I was confused with synced
On Thu, Apr 4, 2024 at 5:53 PM Bharath Rupireddy
wrote:
>
> On Thu, Apr 4, 2024 at 4:35 PM Amit Kapila wrote:
> >
> > > Thanks for the changes. v34-0001 LGTM.
> >
> > I was doing a final review before pushing 0001 and found that
> > 'inactive_since' could be set twice during startup after promoti
On Thu, Apr 4, 2024 at 4:35 PM Amit Kapila wrote:
>
> > Thanks for the changes. v34-0001 LGTM.
>
> I was doing a final review before pushing 0001 and found that
> 'inactive_since' could be set twice during startup after promotion,
> once while restoring slots and then via ShutDownSlotSync(). The r
On Thu, Apr 4, 2024 at 11:12 AM Bharath Rupireddy
wrote:
>
> On Thu, Apr 4, 2024 at 10:48 AM Amit Kapila wrote:
> >
> > The v33-0001 looks good to me. I have made minor changes in the
> > comments/commit message and removed one part of the test which was a
> > bit confusing and didn't seem to add
On Thu, Apr 4, 2024 at 5:36 PM Amit Kapila wrote:
>
> On Thu, Apr 4, 2024 at 1:32 PM Masahiko Sawada wrote:
> >
> > On Thu, Apr 4, 2024 at 1:34 PM Bharath Rupireddy
> > wrote:
> > >
> > > On Thu, Apr 4, 2024 at 9:42 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > @@ -1368,6 +1416,7 @@ ShutDown
On Thu, Apr 4, 2024 at 1:32 PM Masahiko Sawada wrote:
>
> On Thu, Apr 4, 2024 at 1:34 PM Bharath Rupireddy
> wrote:
> >
> > On Thu, Apr 4, 2024 at 9:42 AM Masahiko Sawada
> > wrote:
> > >
> > > @@ -1368,6 +1416,7 @@ ShutDownSlotSync(void)
> > > if (SlotSyncCtx->pid == InvalidPid)
> > >
On Thu, Apr 4, 2024 at 1:34 PM Bharath Rupireddy
wrote:
>
> On Thu, Apr 4, 2024 at 9:42 AM Masahiko Sawada wrote:
> >
> > @@ -1368,6 +1416,7 @@ ShutDownSlotSync(void)
> > if (SlotSyncCtx->pid == InvalidPid)
> > {
> > SpinLockRelease(&SlotSyncCtx->mutex);
> > + update_synced_
On Thu, Apr 4, 2024 at 10:48 AM Amit Kapila wrote:
>
> The v33-0001 looks good to me. I have made minor changes in the
> comments/commit message and removed one part of the test which was a
> bit confusing and didn't seem to add much value. Let me know what you
> think of the attached?
Thanks for
On Wed, Apr 3, 2024 at 8:28 PM Bharath Rupireddy
wrote:
>
> On Wed, Apr 3, 2024 at 6:46 PM Bertrand Drouvot
> wrote:
> >
> > Just one comment on v32-0001:
> >
> > +# Synced slot on the standby must get its own inactive_since.
> > +is( $standby1->safe_psql(
> > + 'postgres',
> > +
On Thu, Apr 4, 2024 at 9:42 AM Masahiko Sawada wrote:
>
> @@ -1368,6 +1416,7 @@ ShutDownSlotSync(void)
> if (SlotSyncCtx->pid == InvalidPid)
> {
> SpinLockRelease(&SlotSyncCtx->mutex);
> + update_synced_slots_inactive_since();
> return;
> }
> SpinLockRelease(&
On Wed, Apr 3, 2024 at 11:58 PM Bharath Rupireddy
wrote:
>
>
> Please find the attached v33 patches.
@@ -1368,6 +1416,7 @@ ShutDownSlotSync(void)
if (SlotSyncCtx->pid == InvalidPid)
{
SpinLockRelease(&SlotSyncCtx->mutex);
+ update_synced_slots_inactive_since();
retur
Hi,
On Wed, Apr 03, 2024 at 08:28:04PM +0530, Bharath Rupireddy wrote:
> On Wed, Apr 3, 2024 at 6:46 PM Bertrand Drouvot
> wrote:
> >
> > Just one comment on v32-0001:
> >
> > +# Synced slot on the standby must get its own inactive_since.
> > +is( $standby1->safe_psql(
> > + 'postgr
On Wed, Apr 3, 2024 at 6:46 PM Bertrand Drouvot
wrote:
>
> Just one comment on v32-0001:
>
> +# Synced slot on the standby must get its own inactive_since.
> +is( $standby1->safe_psql(
> + 'postgres',
> + "SELECT '$inactive_since_on_primary'::timestamptz <=
> '$inactiv
Hi,
On Wed, Apr 03, 2024 at 05:12:12PM +0530, Bharath Rupireddy wrote:
> On Wed, Apr 3, 2024 at 4:19 PM Amit Kapila wrote:
> >
> > + 'postgres',
> > + "SELECT '$inactive_since_on_primary'::timestamptz <
> > '$inactive_since_on_standby'::timestamptz AND
> > + '$inactive_since_on_standby'::timestam
On Wed, Apr 3, 2024 at 12:20 PM Bertrand Drouvot
wrote:
>
> > Please find the attached v31 patches implementing the above idea:
>
> Some comments related to v31-0001:
>
> === testing the behavior
>
> T1 ===
>
> > - synced slots get their on inactive_since just like any other slot
>
> It behaves as
On Wed, Apr 3, 2024 at 2:58 PM shveta malik wrote:
>
> On Wed, Apr 3, 2024 at 11:17 AM Bharath Rupireddy
> wrote:
> >
> > On Wed, Apr 3, 2024 at 8:38 AM shveta malik wrote:
> > >
> > > > > Or a simple solution is that the slotsync worker updates
> > > > > inactive_since as it does for non-synced
On Wed, Apr 3, 2024 at 12:20 PM Bertrand Drouvot
wrote:
>
> On Wed, Apr 03, 2024 at 11:17:41AM +0530, Bharath Rupireddy wrote:
> > On Wed, Apr 3, 2024 at 8:38 AM shveta malik wrote:
> > >
> > > > > Or a simple solution is that the slotsync worker updates
> > > > > inactive_since as it does for no
On Wed, Apr 3, 2024 at 11:17 AM Bharath Rupireddy
wrote:
>
> On Wed, Apr 3, 2024 at 8:38 AM shveta malik wrote:
> >
> > > > Or a simple solution is that the slotsync worker updates
> > > > inactive_since as it does for non-synced slots, and disables
> > > > timeout-based slot invalidation for syn
Hi,
On Wed, Apr 03, 2024 at 11:17:41AM +0530, Bharath Rupireddy wrote:
> On Wed, Apr 3, 2024 at 8:38 AM shveta malik wrote:
> >
> > > > Or a simple solution is that the slotsync worker updates
> > > > inactive_since as it does for non-synced slots, and disables
> > > > timeout-based slot invalida
Hi,
On Wed, Apr 03, 2024 at 11:17:41AM +0530, Bharath Rupireddy wrote:
> On Wed, Apr 3, 2024 at 8:38 AM shveta malik wrote:
> >
> > > > Or a simple solution is that the slotsync worker updates
> > > > inactive_since as it does for non-synced slots, and disables
> > > > timeout-based slot invalida
On Wed, Apr 3, 2024 at 8:38 AM shveta malik wrote:
>
> > > Or a simple solution is that the slotsync worker updates
> > > inactive_since as it does for non-synced slots, and disables
> > > timeout-based slot invalidation for synced slots.
>
> I like this idea better, it takes care of such a case t
On Tue, Apr 2, 2024 at 11:58 AM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Tue, Apr 02, 2024 at 12:07:54PM +0900, Masahiko Sawada wrote:
> > On Mon, Apr 1, 2024 at 12:18 PM Bharath Rupireddy
> >
> > FWIW, coming to this thread late, I think that the inactive_since
> > should not be synchronized from t
Hi,
On Tue, Apr 02, 2024 at 12:41:35PM +0530, Bharath Rupireddy wrote:
> On Tue, Apr 2, 2024 at 11:58 AM Bertrand Drouvot
> wrote:
> >
> > > Or a simple solution is that the slotsync worker updates
> > > inactive_since as it does for non-synced slots, and disables
> > > timeout-based slot invalid
On Tue, Apr 2, 2024 at 11:58 AM Bertrand Drouvot
wrote:
>
> > Or a simple solution is that the slotsync worker updates
> > inactive_since as it does for non-synced slots, and disables
> > timeout-based slot invalidation for synced slots.
>
> Yeah, I think the main question to help us decide is: do
Hi,
On Tue, Apr 02, 2024 at 12:07:54PM +0900, Masahiko Sawada wrote:
> On Mon, Apr 1, 2024 at 12:18 PM Bharath Rupireddy
>
> FWIW, coming to this thread late, I think that the inactive_since
> should not be synchronized from the primary. The wall clocks are
> different on the primary and the stan
On Mon, Apr 1, 2024 at 12:18 PM Bharath Rupireddy
wrote:
>
> On Fri, Mar 29, 2024 at 9:39 AM Amit Kapila wrote:
> >
> > Commit message states: "why we can't just update inactive_since for
> > synced slots on the standby with the value received from remote slot
> > on the primary. This is consiste
Hi,
On Sun, Mar 31, 2024 at 10:25:46AM +0530, Bharath Rupireddy wrote:
> On Thu, Mar 28, 2024 at 3:13 PM Bertrand Drouvot
> wrote:
> > I think in this case it should always reflect the value from the primary (so
> > that one can understand why it is invalidated).
>
> I'll come back to this as so
Hi,
On Mon, Apr 01, 2024 at 08:47:59AM +0530, Bharath Rupireddy wrote:
> On Fri, Mar 29, 2024 at 9:39 AM Amit Kapila wrote:
> >
> > Commit message states: "why we can't just update inactive_since for
> > synced slots on the standby with the value received from remote slot
> > on the primary. This
Hi,
On Mon, Apr 01, 2024 at 09:04:43AM +0530, Amit Kapila wrote:
> On Fri, Mar 29, 2024 at 6:17 PM Bertrand Drouvot
> wrote:
> >
> > On Fri, Mar 29, 2024 at 03:03:01PM +0530, Amit Kapila wrote:
> > > On Fri, Mar 29, 2024 at 11:49 AM Bertrand Drouvot
> > > wrote:
> > > >
> > > > On Fri, Mar 29, 2
On Fri, Mar 29, 2024 at 6:17 PM Bertrand Drouvot
wrote:
>
> On Fri, Mar 29, 2024 at 03:03:01PM +0530, Amit Kapila wrote:
> > On Fri, Mar 29, 2024 at 11:49 AM Bertrand Drouvot
> > wrote:
> > >
> > > On Fri, Mar 29, 2024 at 09:39:31AM +0530, Amit Kapila wrote:
> > > >
> > > > Commit message states:
On Fri, Mar 29, 2024 at 9:39 AM Amit Kapila wrote:
>
> Commit message states: "why we can't just update inactive_since for
> synced slots on the standby with the value received from remote slot
> on the primary. This is consistent with any other slot parameter i.e.
> all of them are synced from th
On Thu, Mar 28, 2024 at 3:13 PM Bertrand Drouvot
wrote:
>
> Regarding 0002:
Thanks for reviewing it.
> Some testing:
>
> T1 ===
>
> When the slot is invalidated on the primary, then the reason is propagated to
> the sync slot (if any). That's fine but we are loosing the inactive_since on
> the
Hi,
On Fri, Mar 29, 2024 at 03:03:01PM +0530, Amit Kapila wrote:
> On Fri, Mar 29, 2024 at 11:49 AM Bertrand Drouvot
> wrote:
> >
> > On Fri, Mar 29, 2024 at 09:39:31AM +0530, Amit Kapila wrote:
> > >
> > > Commit message states: "why we can't just update inactive_since for
> > > synced slots on
On Fri, Mar 29, 2024 at 11:49 AM Bertrand Drouvot
wrote:
>
> On Fri, Mar 29, 2024 at 09:39:31AM +0530, Amit Kapila wrote:
> >
> > Commit message states: "why we can't just update inactive_since for
> > synced slots on the standby with the value received from remote slot
> > on the primary. This is
Hi,
On Fri, Mar 29, 2024 at 09:39:31AM +0530, Amit Kapila wrote:
> On Wed, Mar 27, 2024 at 9:00 PM Bharath Rupireddy
> wrote:
> >
> >
> > Thanks. I'm attaching v29 patches. 0001 managing inactive_since on the
> > standby for sync slots.
> >
>
> Commit message states: "why we can't just update in
On Wed, Mar 27, 2024 at 9:00 PM Bharath Rupireddy
wrote:
>
>
> Thanks. I'm attaching v29 patches. 0001 managing inactive_since on the
> standby for sync slots.
>
Commit message states: "why we can't just update inactive_since for
synced slots on the standby with the value received from remote slo
Hi,
On Wed, Mar 27, 2024 at 09:00:37PM +0530, Bharath Rupireddy wrote:
> standby for sync slots. 0002 implementing inactive timeout GUC based
> invalidation mechanism.
>
> Please have a look.
Thanks!
Regarding 0002:
Some testing:
T1 ===
When the slot is invalidated on the primary, then the r
On Wed, Mar 27, 2024 at 9:00 PM Bharath Rupireddy
wrote:
>
> Thanks. I'm attaching v29 patches. 0001 managing inactive_since on the
> standby for sync slots. 0002 implementing inactive timeout GUC based
> invalidation mechanism.
>
> Please have a look.
Thanks for the patches. v29-001 looks good t
Hi,
On Wed, Mar 27, 2024 at 09:00:37PM +0530, Bharath Rupireddy wrote:
> On Wed, Mar 27, 2024 at 6:54 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > On Wed, Mar 27, 2024 at 05:55:05PM +0530, Bharath Rupireddy wrote:
> > > On Wed, Mar 27, 2024 at 3:42 PM Bertrand Drouvot
> > > Please see the att
On Wed, Mar 27, 2024 at 6:54 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Mar 27, 2024 at 05:55:05PM +0530, Bharath Rupireddy wrote:
> > On Wed, Mar 27, 2024 at 3:42 PM Bertrand Drouvot
> > Please see the attached v28 patch.
>
> Thanks!
>
> 1 === sorry I missed it in the previous review
>
>
Hi,
On Wed, Mar 27, 2024 at 05:55:05PM +0530, Bharath Rupireddy wrote:
> On Wed, Mar 27, 2024 at 3:42 PM Bertrand Drouvot
> Please see the attached v28 patch.
Thanks!
1 === sorry I missed it in the previous review
if (!(RecoveryInProgress() && slot->data.synced))
+ {
On Wed, Mar 27, 2024 at 3:42 PM Bertrand Drouvot
wrote:
>
> 1 ===
>
> My proposal (in text) but feel free to reword it:
>
> Note that the slots on the standbys that are being synced from a
> primary server (whose synced field is true), will get the inactive_since value
> from the corresponding rem
On Wed, Mar 27, 2024 at 2:55 PM Bharath Rupireddy
wrote:
>
> On Wed, Mar 27, 2024 at 11:39 AM shveta malik wrote:
> >
> > Thanks for the patch. Few trivial things:
>
> Thanks for reviewing.
>
> > --
> > 1)
> > system-views.sgml:
> >
> > a) "Note that the slots" --> "Note that the slots on
Hi,
On Wed, Mar 27, 2024 at 02:55:17PM +0530, Bharath Rupireddy wrote:
> Please check the attached v27 patch which also has Bertrand's comment
> on deduplicating the TAP function. I've now moved it to Cluster.pm.
Thanks!
1 ===
+Note that the slots on the standbys that are being synced f
On Wed, Mar 27, 2024 at 11:39 AM shveta malik wrote:
>
> Thanks for the patch. Few trivial things:
Thanks for reviewing.
> --
> 1)
> system-views.sgml:
>
> a) "Note that the slots" --> "Note that the slots on the standbys,"
> --it is good to mention "standbys" as synced could be true on
On Wed, Mar 27, 2024 at 11:05 AM Bharath Rupireddy
wrote:
>
> Fixed an issue in synchronize_slots where DatumGetLSN is being used in
> place of DatumGetTimestampTz. Found this via CF bot member [1], not on
> my dev system.
>
> Please find the attached v6 patch.
Thanks for the patch. Few trivial t
Hi,
On Wed, Mar 27, 2024 at 10:08:33AM +0530, Bharath Rupireddy wrote:
> On Tue, Mar 26, 2024 at 11:22 PM Bertrand Drouvot
> wrote:
> >
> > - if (!(RecoveryInProgress() && slot->data.synced))
> > + if (!(InRecovery && slot->data.synced))
> > slo
On Wed, Mar 27, 2024 at 10:08 AM Bharath Rupireddy
wrote:
>
> Please find the attached v25-0001 (made this 0001 patch now as
> inactive_since patch is committed) patch with the above changes.
Fixed an issue in synchronize_slots where DatumGetLSN is being used in
place of DatumGetTimestampTz. Foun
On Tue, Mar 26, 2024 at 6:05 PM Bertrand Drouvot
wrote:
>
>
> > We can think on that later if we really need another
> > field which give us sync time.
>
> I think that calling GetCurrentTimestamp() so frequently could be too costly,
> so
> I'm not sure we should.
Agreed.
> > In my second appro
On Wed, Mar 27, 2024 at 10:24 AM shveta malik wrote:
>
> On Wed, Mar 27, 2024 at 10:22 AM Amit Kapila wrote:
> >
> > On Wed, Mar 27, 2024 at 10:08 AM Bharath Rupireddy
> > wrote:
> > >
> > > On Tue, Mar 26, 2024 at 11:22 PM Bertrand Drouvot
> > > wrote:
> > >
> > > > 3)
> > > > update_synced_sl
1 - 100 of 266 matches
Mail list logo
