Re: Missing LWLock protection in pgstat_reset_replslot()

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
> <bertranddrouvot.pg@gmail.com> 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 as we collect stats only for logical
> -        * slots.
> +        * Reset stats if it is a logical slot. Nothing to do for physical slots
> +        * as we collect stats only for logical slots.
>          */
> -       if (SlotIsPhysical(slot))
> -       {
> -               LWLockRelease(ReplicationSlotControlLock);
> -               return;
> -       }
> -
> -       /* reset this one entry */
> -       pgstat_reset(PGSTAT_KIND_REPLSLOT, InvalidOid,
> -                                ReplicationSlotIndex(slot));
> +       if (SlotIsLogical(slot))
> +               pgstat_reset(PGSTAT_KIND_REPLSLOT, InvalidOid,
> +                                        ReplicationSlotIndex(slot));
> 
>         LWLockRelease(ReplicationSlotControlLock);
> 

Yeah, it's easier to read and probably reduce the pgstat_replslot.o object file
size a bit for non optimized build.

> Something similar in pgstat_fetch_replslot() perhaps?

Yeah, all of the above done in v3 attached.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com