Re: Replication slot stats misgivings

Masahiko Sawada <sawada.mshk@gmail.com>

From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Date: 2021-04-21T09:15:36Z
Lists: pgsql-hackers
On Wed, Apr 21, 2021 at 3:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Apr 20, 2021 at 7:54 PM Masahiko Sawada <sawada.mshk@gmail.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/



Commits

  1. Doc: Update logical decoding stats information.

  2. Fix tests for replication slots stats.

  3. Update replication statistics after every stream/spill.

  4. Fix the computation of slot stats for 'total_bytes'.

  5. Another try to fix the test case added by commit f5fc2f5b23.

  6. Use HTAB for replication slot statistics.

  7. Fix test case added by commit f5fc2f5b23.

  8. Add information of total data processed to replication slot stats.

  9. Use NameData datatype for slotname in stats.