On Wed, Apr 21, 2021 at 2:37 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, Apr 21, 2021 at 12:50 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Tue, Apr 20, 2021 at 7:54 PM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > > I have one question: > > > > + /* > > + * Create the replication slot stats hash table if we don't have > > + * it already. > > + */ > > + if (replSlotStats == NULL) > > { > > - if (namestrcmp(&replSlotStats[i].slotname, name) == 0) > > - return i; /* found */ > > + HASHCTL hash_ctl; > > + > > + hash_ctl.keysize = sizeof(NameData); > > + hash_ctl.entrysize = sizeof(PgStat_StatReplSlotEntry); > > + hash_ctl.hcxt = pgStatLocalContext; > > + > > + replSlotStats = hash_create("Replication slots hash", > > + PGSTAT_REPLSLOT_HASH_SIZE, > > + &hash_ctl, > > + HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); > > } > > > > It seems to me that the patch is always creating a hash table in > > pgStatLocalContext? AFAIU, we need to create it in pgStatLocalContext > > when we read stats via backend_read_statsfile so that we can clear it > > at the end of the transaction. The db/function stats seems to be doing > > the same. Is there a reason why here we need to always create it in > > pgStatLocalContext? > > I wanted to avoid creating the hash table if there is no replication > slot. But as you pointed out, we create the hash table even when > lookup (i.g., create_it is false), which is bad. So I think we can > have pgstat_get_replslot_entry() return NULL without creating the hash > table if the hash table is NULL and create_it is false so that backend > processes don’t create the hash table, not via > backend_read_statsfile(). Or another idea would be to always create > the hash table in pgstat_read_statsfiles(). That way, it would > simplify the code but could waste the memory if there is no > replication slot. >
If you create it after reading 'R' message as we do in the case of 'D' message then it won't waste any memory. So probably creating in pgstat_read_statsfiles() would be better unless you see some other problem with that. -- With Regards, Amit Kapila.