Re: Replication slot stats misgivings

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Subject: Re: Replication slot stats misgivings
Date: 2021-04-21 09:15:36
Message-ID: CAD21AoA4rB5nYeF3n121+DBNG_YN0n=WaR=Uw0bio0iHU5Xfyw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 21, 2021 at 3:09 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Apr 20, 2021 at 7:54 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> >
> > I've attached the patch. In addition to the test Vignesh prepared, I
> > added one test for the message for creating a slot that checks if the
> > statistics are initialized after re-creating the same name slot.
> >
>
> I am not sure how much useful your new test is because you are testing
> it for slot name for which we have removed the slot file. It is not
> related to stat messages this patch is sending. I think we can leave
> that for now.

I might be missing something but I think the test is related to the
message for creating a slot that initializes all counters. No? If
there is no that message, we will end up getting old stats if a
message for dropping slot gets lost (simulated by dropping slot file)
and the same name slot is created.

> One other minor comment:
>
> - * create the statistics for the replication slot.
> + * create the statistics for the replication slot. In the cases where the
> + * message for dropping the old slot gets lost and a slot with the same
> + * name is created, since the stats will be initialized by the message
> + * for creating the slot the statistics are not accumulated into the
> + * old slot unless the messages for both creating and dropping slots with
> + * the same name got lost. Just in case it happens, the user can reset
> + * the particular stats by pg_stat_reset_replication_slot().
>
> I think we can change it to something like: " XXX In case, the
> messages for creation and drop slot of the same name get lost and
> create happens before (auto)vacuum cleans up the dead slot, the stats
> will be accumulated into the old slot. One can imagine having OIDs for
> each slot to avoid the accumulation of stats but that doesn't seem
> worth doing as in practice this won't happen frequently.". Also, I am
> not sure after your recent change whether it is a good idea to mention
> something in docs. What do you think?

Both points make sense to me. I'll update the comment and remove the
mention in the doc in the next version patch.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Oleg Bartunov 2021-04-21 09:16:30 Re: ML-based indexing ("The Case for Learned Index Structures", a paper from Google)
Previous Message Ian Zagorskikh 2021-04-21 09:08:09 Re: libpq compression