Re: Avoid unnecessary ReplicationSlotControl lwlock acquistion

Hi,

On 2020-08-25 12:00:47 +0900, Masahiko Sawada wrote:
> On Tue, 25 Aug 2020 at 11:42, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > Hi,
> >
> > On August 24, 2020 7:38:39 PM PDT, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com> wrote:
> > >Hi all,
> > >
> > >While testing with DTrace, I realized we acquire
> > >ReplicationSlotControl lwlock at some places even when
> > >max_replication_slots is set to 0. For instance, we call
> > >ReplicationSlotCleanup() within PostgresMian() when an error happens
> > >and acquire ReplicationSlotControl lwlock.
> > >
> > >The attached patch fixes some functions so that we quickly return if
> > >max_replication_slots is set to 0.
> >
> > Why is it worth doing so?
>
> I think we can avoid unnecessary overhead caused by acquiring and
> releasing that lwlock itself. The functions modified by this patch are
> called during error cleanup or checkpoints. For the former case,
> since it’s not a commit path the benefit might not be large on common
> workload but it might help to reduce the latency on a workload whose
> abort rate is relatively high. Also looking at other functions in
> slot.c, other functions also do so. I think these are also for
> preventing unnecessary overhead.

I don't see how these could matter. The error checking path isn't
reached if no slot is acquired. One uncontended lwlock acquisition isn't
measurable compared to the cost of a checkpoint.

Greetings,

Andres Freund