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-22T07:31:24Z
Lists: pgsql-hackers

Attachments

On Thu, Apr 22, 2021 at 3:03 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Apr 22, 2021 at 10:39 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Thu, Apr 22, 2021 at 1:50 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Thu, Apr 22, 2021 at 8:26 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> >
> > > 2.
> > > + if (replSlotStatHash != NULL)
> > > + (void) hash_search(replSlotStatHash,
> > > +    (void *) &(msg->m_slotname),
> > > +    HASH_REMOVE,
> > > +    NULL);
> > >
> > > Why have you changed this part from using NameStr?
> >
> > I thought that since the hash table is created with the key size
> > sizeof(NameData) it's better to use NameData for searching as well.
> >
>
> Fair enough. I think this will give the same result either way.

I've attached the updated version patch.

Besides the review comment from Amit, I changed
pgstat_read_statsfiles() so that it doesn't use
spgstat_get_replslot_entry(). That’s because it was slightly unclear
why we create the hash table beforehand even though we call
pgstat_read_statsfiles() with ‘create’ = true. By this change, the
caller of pgstat_get_replslot_entry() with ‘create’ = true is only
pgstat_recv_replslot(), which makes the code clear and safe since
pgstat_recv_replslot() is used only by the collector. Also, I ran
pgindent on the modified files.

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.