Re: Replication slot stats misgivings

On Mon, Apr 19, 2021 at 9:00 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Apr 16, 2021 at 2:58 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> >
> > 4.
> > +CREATE VIEW pg_stat_replication_slots AS
> > +    SELECT
> > +            s.slot_name,
> > +            s.spill_txns,
> > +            s.spill_count,
> > +            s.spill_bytes,
> > +            s.stream_txns,
> > +            s.stream_count,
> > +            s.stream_bytes,
> > +            s.total_txns,
> > +            s.total_bytes,
> > +            s.stats_reset
> > +    FROM pg_replication_slots as r,
> > +        LATERAL pg_stat_get_replication_slot(slot_name) as s
> > +    WHERE r.datoid IS NOT NULL; -- excluding physical slots
> > ..
> > ..
> >
> > -/* Get the statistics for the replication slots */
> > +/* Get the statistics for the replication slot */
> >  Datum
> > -pg_stat_get_replication_slots(PG_FUNCTION_ARGS)
> > +pg_stat_get_replication_slot(PG_FUNCTION_ARGS)
> >  {
> >  #define PG_STAT_GET_REPLICATION_SLOT_COLS 10
> > - ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
> > + text *slotname_text = PG_GETARG_TEXT_P(0);
> > + NameData slotname;
> >
> > I think with the above changes getting all the slot stats has become
> > much costlier. Is there any reason why can't we get all the stats from
> > the new hash_table in one shot and return them to the user?
>
> I think the advantage of this approach would be that it can avoid
> showing the stats for already-dropped slots. Like other statistics
> views such as pg_stat_all_tables and pg_stat_all_functions, searching
> the stats by the name got from pg_replication_slots can show only
> available slot stats even if the hash table has garbage slot stats.
>

Sounds reasonable. However, if the create_slot message is missed, it
will show an empty row for it. See below:

postgres=# select slot_name, total_txns from pg_stat_replication_slots;
 slot_name | total_txns
-----------+------------
 s1        |          0
 s2        |          0
           |
(3 rows)

Here, I have manually via debugger skipped sending the create_slot
message for the third slot and we are showing an empty for it. This
won't happen for pg_stat_all_tables, as it will set 0 or other initial
values in such a case. I think we need to address this case.

> Given that pg_stat_replication_slots doesn’t show garbage slot stats
> even if it has, I thought we can avoid checking those garbage stats
> frequently. It should not essentially be a problem for the hash table
> to have entries up to max_replication_slots regardless of live or
> already-dropped.
>

Yeah, but I guess we still might not save much by not doing it,
especially because for the other cases like tables/functions, we are
doing it without any threshold limit.

-- 
With Regards,
Amit Kapila.