Re: Replication slot stats misgivings

On Tue, Apr 27, 2021 at 9:17 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Apr 27, 2021 at 11:31 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > > > And I think there is
> > > > also a risk to increase shared memory when we want to add other
> > > > statistics in the future.
> > > >
> > >
> > > Yeah, so do you think it is not a good idea to store stats in
> > > ReplicationSlot? Actually storing them in a slot makes it easier to
> > > send them during ReplicationSlotRelease which is quite helpful if the
> > > replication is interrupted due to some reason. Or the other idea was
> > > that we send stats every time we stream or spill changes.
> >
> > We use around 64 bytes of shared memory to store the statistics
> > information per slot, I'm not sure if this is a lot of memory. If this
> > memory is fine, then I felt the approach to store stats seems fine. If
> > that memory is too much then we could use the other approach to update
> > stats when we stream or spill the changes as suggested by Amit.
>
> I agree that makes it easier to send slot stats during
> ReplicationSlotRelease() but I'd prefer to avoid storing data that
> doesn't need to be shared in the shared buffer if possible.
>

Sounds reasonable and we might add some stats in the future so that
will further increase the usage of shared memory.

> And those
> counters are not used by physical slots at all. If sending slot stats
> every time we stream or spill changes doesn't affect the system much,
> I think it's better than having slot stats in the shared memory.
>

As the minimum size of logical_decoding_work_mem is 64KB, so in the
worst case, we will send stats after decoding that many changes. I
don't think it would impact too much considering that we need to spill
or stream those many changes.  If it concerns any users they can
always increase logical_decoding_work_mem. The default value is 64MB
at which point, I don't think it will matter sending the stats.

> Also, not sure it’s better but another idea would be to make the slot
> stats a global variable like pgBufferUsage and use it during decoding.
>

Hmm, I think it is better to avoid global variables if possible.

> Or we can set a proc-exit callback? But to be honest, I'm not sure
> which approach we should go with. Those approaches have proc and cons.
>

I think we can try the first approach listed here which is to send
stats each time we spill or stream.

-- 
With Regards,
Amit Kapila.