On Tue, Apr 02, 2024 at 03:18:24PM -0400, Tom Lane wrote:
> I've closed the CF entry for this [1] as committed. Please re-open
> it if there was something left to do here.
>
> [1] https://commitfest.postgresql.org/47/4878/
Thanks, I was not aware of this one.
--
Michael
signature.asc
Michael Paquier writes:
> On Fri, Mar 08, 2024 at 07:46:26PM +0900, Michael Paquier wrote:
>> Sounds good to me.
> I've applied the patch of this thread as b36fbd9f8da1, though I did
> not see a huge point in backpatching as at the end this is just a
> consistency improvement.
I've closed the
On Fri, Mar 08, 2024 at 07:46:26PM +0900, Michael Paquier wrote:
> Sounds good to me.
I've applied the patch of this thread as b36fbd9f8da1, though I did
not see a huge point in backpatching as at the end this is just a
consistency improvement.
--
Michael
signature.asc
Description: PGP
On Fri, Mar 08, 2024 at 10:26:21AM +, Bertrand Drouvot wrote:
> Yeah, good point: I'll create a dedicated patch for that.
Sounds good to me.
> Note that currently pgstat_drop_replslot() would not satisfy this new Assert
> when being called from InvalidatePossiblyObsoleteSlot(). I think this
Hi,
On Thu, Mar 07, 2024 at 02:17:53PM +0900, Michael Paquier wrote:
> On Wed, Mar 06, 2024 at 09:05:59AM +, Bertrand Drouvot wrote:
> > Yeah, all of the above done in v3 attached.
>
> In passing.. pgstat_create_replslot() and pgstat_drop_replslot() rely
> on the assumption that the LWLock
On Thu, Mar 07, 2024 at 11:30:55AM +0530, shveta malik wrote:
> It slightly improves the chances. pgstat_fetch_replslot is only
> called from pg_stat_get_replication_slot(), I thought it might be
> better to acquire lock before we call pgstat_fetch_replslot and
> release once we are done copying
On Thu, Mar 7, 2024 at 11:12 AM Michael Paquier wrote:
>
> Yeah, it is possible that what you retrieve from
> pgstat_fetch_replslot() does not refer exactly to the slot's content
> under concurrent activity, but you cannot protect a single scan of
> pg_stat_replication_slots as of an effect of
On Thu, Mar 07, 2024 at 10:57:28AM +0530, shveta malik wrote:
> --It can happen in a small window in pg_stat_get_replication_slot()
> when we are consuming the return values of pgstat_fetch_replslot
> (using slotent).
Yeah, it is possible that what you retrieve from
pgstat_fetch_replslot() does
On Wed, Mar 6, 2024 at 2:36 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Mar 06, 2024 at 10:24:46AM +0530, shveta malik wrote:
> > On Tue, Mar 5, 2024 at 6:52 PM Bertrand Drouvot
> > wrote:
> > Thanks. Can we try to get rid of multiple LwLockRelease in
> > pgstat_reset_replslot(). Is this any
On Wed, Mar 06, 2024 at 09:05:59AM +, Bertrand Drouvot wrote:
> Yeah, all of the above done in v3 attached.
Interesting, so this relies on the slot index to ensure the unicity of
the stat entries. And if the entry pointing to this ID is updated
we may refer to just incorrect data.
The
Hi,
On Wed, Mar 06, 2024 at 10:24:46AM +0530, shveta malik wrote:
> On Tue, Mar 5, 2024 at 6:52 PM Bertrand Drouvot
> wrote:
> Thanks. Can we try to get rid of multiple LwLockRelease in
> pgstat_reset_replslot(). Is this any better?
>
> /*
> -* Nothing to do for physical slots
On Tue, Mar 5, 2024 at 6:52 PM Bertrand Drouvot
wrote:
>
> > /*
> > * Nothing to do for physical slots as we collect stats only for logical
> > * slots.
> > */
> > if (SlotIsPhysical(slot))
> > return;
>
> D'oh! Thanks! Fixed in v2 shared up-thread.
Thanks. Can we try to get rid of multiple
Hi,
On Tue, Mar 05, 2024 at 02:19:19PM +0530, shveta malik wrote:
> On Tue, Mar 5, 2024 at 1:25 PM Heikki Linnakangas wrote:
>
> > SearchNamedReplicationSlot() will also acquire the lock in LW_SHARED
> > mode, when you pass need_lock=true. So that at least should be changed
> > to false.
> >
>
Hi,
On Tue, Mar 05, 2024 at 09:55:32AM +0200, Heikki Linnakangas wrote:
> On 01/03/2024 12:15, Bertrand Drouvot wrote:
> > Hi hackers,
> >
> > I think that pgstat_reset_replslot() is missing LWLock protection. Indeed,
> > we
> > don't have any guarantee that th
On Tue, Mar 5, 2024 at 1:25 PM Heikki Linnakangas wrote:
> SearchNamedReplicationSlot() will also acquire the lock in LW_SHARED
> mode, when you pass need_lock=true. So that at least should be changed
> to false.
>
Also don't we need to release the lock when we return here:
/*
* Nothing to do
On 01/03/2024 12:15, Bertrand Drouvot wrote:
Hi hackers,
I think that pgstat_reset_replslot() is missing LWLock protection. Indeed, we
don't have any guarantee that the slot is active (then preventing it to be
dropped/recreated) when this function is executed.
Yes, so it seems at quick glance
Hi hackers,
I think that pgstat_reset_replslot() is missing LWLock protection. Indeed, we
don't have any guarantee that the slot is active (then preventing it to be
dropped/recreated) when this function is executed.
Attached a patch to add the missing protection.
Regards,
--
Bertrand Drouvot
17 matches
Mail list logo
