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:
> >
> > ---
> >
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
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
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
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
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
+
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
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
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
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 @@
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(>mutex);
> > +
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
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(>mutex);
> + update_synced_slots_inactive_since();
> return;
> }
> SpinLockRelease(>mutex);
> @@
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(>mutex);
+ update_synced_slots_inactive_since();
return;
}
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(
> > +
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 <=
>
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
> > +
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
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
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
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
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
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
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
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
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
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:
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
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
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
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.
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,
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
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
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
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
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
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
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
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
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
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
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
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
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))
> >
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.
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
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)
> > > >
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_slots_inactive_time():
> > >
> > > This assert is removed, is it intentional?
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_slots_inactive_time():
> >
> > This assert is removed, is it intentional?
> > Assert(s->active_pid == 0);
>
> Yes, the slot can get acquired in the
On Tue, Mar 26, 2024 at 11:22 PM Bertrand Drouvot
wrote:
>
> > I'm attaching v24 patches. It implements the above idea proposed
> > upthread for synced slots.
>
> v24-0002
>
> 1 ===
>
> This commit does two things:
> 1) Updates inactive_since for sync slots with the value
>
On Tue, Mar 26, 2024 at 9:59 PM Bharath Rupireddy
wrote:
>
> On Tue, Mar 26, 2024 at 4:35 PM Bharath Rupireddy
> wrote:
> >
> > If we just sync inactive_since value for synced slots while in
> > recovery from the primary, so be it. Why do we need to update it to
> > the current time when the
Hi,
On Tue, Mar 26, 2024 at 09:59:23PM +0530, Bharath Rupireddy wrote:
> On Tue, Mar 26, 2024 at 4:35 PM Bharath Rupireddy
> wrote:
> >
> > If we just sync inactive_since value for synced slots while in
> > recovery from the primary, so be it. Why do we need to update it to
> > the current time
On Tue, Mar 26, 2024 at 4:35 PM Bharath Rupireddy
wrote:
>
> If we just sync inactive_since value for synced slots while in
> recovery from the primary, so be it. Why do we need to update it to
> the current time when the slot is being created? We don't expose slot
> creation time, no? Aren't we
Hi,
On Tue, Mar 26, 2024 at 04:17:53PM +0530, shveta malik wrote:
> On Tue, Mar 26, 2024 at 3:50 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > > I think there may have been some misunderstanding here.
> >
> > Indeed ;-)
> >
> > > But now if I
> > > rethink this, I am fine with
Hi,
On Tue, Mar 26, 2024 at 04:49:18PM +0530, shveta malik wrote:
> On Tue, Mar 26, 2024 at 4:35 PM Bharath Rupireddy
> wrote:
> >
> > On Tue, Mar 26, 2024 at 4:18 PM shveta malik wrote:
> > >
> > > > What about another approach?: inactive_since gives data synced from
> > > > primary for
> > >
On Tue, Mar 26, 2024 at 4:35 PM Bharath Rupireddy
wrote:
>
> On Tue, Mar 26, 2024 at 4:18 PM shveta malik wrote:
> >
> > > What about another approach?: inactive_since gives data synced from
> > > primary for
> > > synced slots and another dedicated field (could be added later...) could
> > >
On Tue, Mar 26, 2024 at 4:18 PM shveta malik wrote:
>
> > What about another approach?: inactive_since gives data synced from primary
> > for
> > synced slots and another dedicated field (could be added later...) could
> > represent what you suggest as the other option.
>
> Yes, okay with me. I
On Tue, Mar 26, 2024 at 3:50 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> > I think there may have been some misunderstanding here.
>
> Indeed ;-)
>
> > But now if I
> > rethink this, I am fine with 'inactive_since' getting synced from
> > primary to standby. But if we do that, we need to add docs
On Tue, Mar 26, 2024 at 7:57 PM Bharath Rupireddy <
bharath.rupireddyforpostg...@gmail.com> wrote:
> Please see the attached v23 patches. I've addressed all the review
> comments received so far from Amit and Shveta.
>
>
In patch 0003:
+ SpinLockAcquire(>mutex);
+ }
+
+
On Tue, Mar 26, 2024 at 3:12 PM Bertrand Drouvot
wrote:
>
> On Tue, Mar 26, 2024 at 02:27:17PM +0530, Bharath Rupireddy wrote:
> > Please use the v22 patch set.
>
> Thanks!
>
> 1 ===
>
> +reset_synced_slots_info(void)
>
> I'm not sure "reset" is the right word, what about
>
Hi,
On Tue, Mar 26, 2024 at 03:17:36PM +0530, shveta malik wrote:
> On Tue, Mar 26, 2024 at 1:54 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > On Tue, Mar 26, 2024 at 01:37:21PM +0530, Amit Kapila wrote:
> > > On Tue, Mar 26, 2024 at 1:15 PM Bertrand Drouvot
> > > wrote:
> > > >
> > > > 2
On Tue, Mar 26, 2024 at 1:54 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Tue, Mar 26, 2024 at 01:37:21PM +0530, Amit Kapila wrote:
> > On Tue, Mar 26, 2024 at 1:15 PM Bertrand Drouvot
> > wrote:
> > >
> > > 2 ===
> > >
> > > It looks like inactive_since is set to the current timestamp on the
> >
Hi,
On Tue, Mar 26, 2024 at 02:27:17PM +0530, Bharath Rupireddy wrote:
> Please use the v22 patch set.
Thanks!
1 ===
+reset_synced_slots_info(void)
I'm not sure "reset" is the right word, what about slot_sync_shutdown_update()?
2 ===
+ for (int i = 0; i < max_replication_slots; i++)
+
On Tue, Mar 26, 2024 at 2:27 PM Bharath Rupireddy
wrote:
>
> >
> > 1)
> > Commti msg:
> >
> > ensures the value is set to current timestamp during the
> > shutdown to help correctly interpret the time if the standby gets
> > promoted without a restart.
> >
> > shutdown --> shutdown of slot sync
On Tue, Mar 26, 2024 at 11:26 AM Amit Kapila wrote:
>
> Review comments on v18_0002 and v18_0005
> ===
>
> 1.
> We have decided to update inactive_since for temporary slots. So,
> unless there is some reason, we should allow inactive_timeout to also
> be set
Hi,
On Tue, Mar 26, 2024 at 01:37:21PM +0530, Amit Kapila wrote:
> On Tue, Mar 26, 2024 at 1:15 PM Bertrand Drouvot
> wrote:
> >
> > 2 ===
> >
> > It looks like inactive_since is set to the current timestamp on the standby
> > each time the sync worker does a cycle:
> >
> > primary:
> >
> >
On Tue, Mar 26, 2024 at 1:15 PM Bertrand Drouvot
wrote:
>
> 2 ===
>
> It looks like inactive_since is set to the current timestamp on the standby
> each time the sync worker does a cycle:
>
> primary:
>
> postgres=# select slot_name,inactive_since from pg_replication_slots where
> failover =
Hi,
On Tue, Mar 26, 2024 at 11:07:51AM +0530, Bharath Rupireddy wrote:
> On Tue, Mar 26, 2024 at 9:30 AM shveta malik wrote:
> > But immediately after promotion, we can not rely on the above check
> > and thus possibility of synced slots invalidation is there. To
> > maintain consistent behavior
On Tue, Mar 26, 2024 at 11:08 AM Bharath Rupireddy
wrote:
>
> On Tue, Mar 26, 2024 at 9:30 AM shveta malik wrote:
> >
> > On Mon, Mar 25, 2024 at 12:43 PM shveta malik
> > wrote:
> > >
> > > I have one concern, for synced slots on standby, how do we disallow
> > > invalidation due to
On Tue, Mar 26, 2024 at 11:36 AM Bertrand Drouvot
wrote:
> >
> > The issue that I can see with your proposal is: what if one synced the slots
> > manually (with pg_sync_replication_slots()) but does not use the sync
> > worker?
> > Then I think ShutDownSlotSync() is not going to help in that
Hi,
On Tue, Mar 26, 2024 at 05:55:11AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Tue, Mar 26, 2024 at 09:30:32AM +0530, shveta malik wrote:
> > On Mon, Mar 25, 2024 at 12:43 PM shveta malik
> > wrote:
> > >
> > > I have one concern, for synced slots on standby, how do we disallow
> > >
On Sun, Mar 24, 2024 at 3:05 PM Bharath Rupireddy
wrote:
>
> I've attached the v18 patch set here. I've also addressed earlier
> review comments from Amit, Ajin Cherian. Note that I've added new
> invalidation mechanism tests in a separate TAP test file just because
> I don't want to clutter or
Hi,
On Tue, Mar 26, 2024 at 09:30:32AM +0530, shveta malik wrote:
> On Mon, Mar 25, 2024 at 12:43 PM shveta malik wrote:
> >
> > I have one concern, for synced slots on standby, how do we disallow
> > invalidation due to inactive-timeout immediately after promotion?
> >
> > For synced slots,
On Tue, Mar 26, 2024 at 9:30 AM shveta malik wrote:
>
> On Mon, Mar 25, 2024 at 12:43 PM shveta malik wrote:
> >
> > I have one concern, for synced slots on standby, how do we disallow
> > invalidation due to inactive-timeout immediately after promotion?
> >
> > For synced slots,
On Tue, Mar 26, 2024 at 1:24 AM Nathan Bossart wrote:
>
>
> On Sun, Mar 24, 2024 at 03:05:44PM +0530, Bharath Rupireddy wrote:
> > This commit particularly lets one specify the inactive_timeout for
> > a slot via SQL functions pg_create_physical_replication_slot and
> >
On Mon, Mar 25, 2024 at 12:43 PM shveta malik wrote:
>
> I have one concern, for synced slots on standby, how do we disallow
> invalidation due to inactive-timeout immediately after promotion?
>
> For synced slots, last_inactive_time and inactive_timeout are both
> set. Let's say I bring down
I apologize that I haven't been able to keep up with this thread for a
while, but I'm happy to see the continued interest in $SUBJECT.
On Sun, Mar 24, 2024 at 03:05:44PM +0530, Bharath Rupireddy wrote:
> This commit particularly lets one specify the inactive_timeout for
> a slot via SQL functions
On Mon, Mar 25, 2024 at 5:10 PM Amit Kapila wrote:
>
> I think we should keep pg_alter_replication_slot() as the last
> priority among the remaining patches for this release. Let's try to
> first finish the primary functionality of inactive_timeout patch.
> Otherwise, I agree that the problem
On Mon, Mar 25, 2024 at 2:40 PM shveta malik wrote:
>
> On Mon, Mar 25, 2024 at 1:37 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > Yeah, and I can see last_inactive_time is moving on the standby (while not
> > the
> > case on the primary), probably due to the sync worker slot
> >
On Mon, Mar 25, 2024 at 3:31 PM Bharath Rupireddy
wrote:
>
> Right. Done that way i.e. not setting the last_inactive_time for slots
> both while releasing the slot and restoring from the disk.
>
> Also, I've added a TAP function to check if the captured times are
> sane per Bertrand's review
On Mon, Mar 25, 2024 at 3:31 PM Bharath Rupireddy
wrote:
>
> Right. Done that way i.e. not setting the last_inactive_time for slots
> both while releasing the slot and restoring from the disk.
>
> Also, I've added a TAP function to check if the captured times are
> sane per Bertrand's review
On Mon, Mar 25, 2024 at 1:37 PM Bertrand Drouvot
wrote:
>
> > > I have one concern, for synced slots on standby, how do we disallow
> > > invalidation due to inactive-timeout immediately after promotion?
> > >
> > > For synced slots, last_inactive_time and inactive_timeout are both
> > > set.
>
>
Hi,
On Mon, Mar 25, 2024 at 02:39:50PM +0530, shveta malik wrote:
> I am listing the concerns raised by me:
> 3) alter replication slot to alter inactive_timout for synced slots on
> standby, should this be allowed?
I don't think it should be allowed.
Regards,
--
Bertrand Drouvot
PostgreSQL
On Mon, Mar 25, 2024 at 1:37 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> Yeah, and I can see last_inactive_time is moving on the standby (while not the
> case on the primary), probably due to the sync worker slot acquisition/release
> which does not seem right.
>
Yes, you are right,
Hi,
On Mon, Mar 25, 2024 at 02:07:21PM +0530, shveta malik wrote:
> On Mon, Mar 25, 2024 at 1:37 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > On Mon, Mar 25, 2024 at 12:59:52PM +0530, Amit Kapila wrote:
> > > On Mon, Mar 25, 2024 at 12:43 PM shveta malik
> > > wrote:
> > > >
> > > > On
On Mon, Mar 25, 2024 at 1:37 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Mon, Mar 25, 2024 at 12:59:52PM +0530, Amit Kapila wrote:
> > On Mon, Mar 25, 2024 at 12:43 PM shveta malik
> > wrote:
> > >
> > > On Mon, Mar 25, 2024 at 11:53 AM shveta malik
> > > wrote:
> > > >
> > > > On Mon, Mar 25,
Hi,
On Mon, Mar 25, 2024 at 12:59:52PM +0530, Amit Kapila wrote:
> On Mon, Mar 25, 2024 at 12:43 PM shveta malik wrote:
> >
> > On Mon, Mar 25, 2024 at 11:53 AM shveta malik
> > wrote:
> > >
> > > On Mon, Mar 25, 2024 at 10:33 AM shveta malik
> > > wrote:
> > > >
> > > > On Sun, Mar 24, 2024
Hi,
On Mon, Mar 25, 2024 at 12:25:21PM +0530, Bharath Rupireddy wrote:
> On Mon, Mar 25, 2024 at 10:28 AM Amit Kapila wrote:
> >
> > On Mon, Mar 25, 2024 at 9:48 AM Amit Kapila wrote:
> > >
> > >
> > > Such a test looks reasonable but shall we add equal to in the second
> > > part of the test
On Mon, Mar 25, 2024 at 12:43 PM shveta malik wrote:
>
> On Mon, Mar 25, 2024 at 11:53 AM shveta malik wrote:
> >
> > On Mon, Mar 25, 2024 at 10:33 AM shveta malik
> > wrote:
> > >
> > > On Sun, Mar 24, 2024 at 3:06 PM Bharath Rupireddy
> > > wrote:
> > > >
> > > > I've attached the v18 patch
1 - 100 of 224 matches
Mail list logo
