Re: Replication slot stats misgivings

On Thu, Apr 22, 2021 at 3:03 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Apr 22, 2021 at 10:39 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Thu, Apr 22, 2021 at 1:50 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Apr 22, 2021 at 8:26 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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/