PgStat_KindInfo.named_on_disk not required in shared stats

Started by Michael Paquieralmost 2 years ago4 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,
(Relevant folks in CC.)

While hacking on the area of pgstat_*.c, I have noticed the existence
of named_on_disk in PgStat_KindInfo, that is here to track the fact
that replication slots are a particular case in the PgStat_HashKey for
the dshash table of the stats because this kind of stats requires a
mapping between the replication slot name and the hash key.

As far as I can see, this field is not required and is used nowhere,
because the code relies on the existence of the to_serialized_name and
from_serialized_name callbacks to do the mapping.

Wouldn't it make sense to remove it? This field is defined since
5891c7a8ed8f that introduced the shmem stats, and has never been used
since.

This frees an extra bit in PgStat_KindInfo, which is going to help me
a bit with what I'm doing with this area of the code while keeping the
structure size the same.

Thoughts?
--
Michael

Attachments:

0001-Remove-PgStat_KindInfo.named_on_disk.patchtext/x-diff; charset=us-asciiDownload+1-9
#2Andres Freund
andres@anarazel.de
In reply to: Michael Paquier (#1)
Re: PgStat_KindInfo.named_on_disk not required in shared stats

Hi,

On 2024-06-07 14:07:33 +0900, Michael Paquier wrote:

While hacking on the area of pgstat_*.c, I have noticed the existence
of named_on_disk in PgStat_KindInfo, that is here to track the fact
that replication slots are a particular case in the PgStat_HashKey for
the dshash table of the stats because this kind of stats requires a
mapping between the replication slot name and the hash key.

As far as I can see, this field is not required and is used nowhere,
because the code relies on the existence of the to_serialized_name and
from_serialized_name callbacks to do the mapping.

Wouldn't it make sense to remove it? This field is defined since
5891c7a8ed8f that introduced the shmem stats, and has never been used
since.

Yes, makes sense. Looks we changed direction during development a bunch of times...q

This frees an extra bit in PgStat_KindInfo, which is going to help me
a bit with what I'm doing with this area of the code while keeping the
structure size the same.

Note it's just a single bit, not a full byte. So unless you need precisely 30
bits, rather than 29, I don't really see why it'd help? And i don't see a
reason to strictly keep the structure size the same.

Greetings,

Andres Freund

#3Michael Paquier
michael@paquier.xyz
In reply to: Andres Freund (#2)
Re: PgStat_KindInfo.named_on_disk not required in shared stats

On Fri, Jun 07, 2024 at 08:30:06AM -0700, Andres Freund wrote:

Yes, makes sense. Looks we changed direction during development a bunch of times...q

Thanks for looking, Andres! I guess I'll just apply that once v18
opens up.
--
Michael

#4Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#3)
Re: PgStat_KindInfo.named_on_disk not required in shared stats

On Sat, Jun 08, 2024 at 07:31:21AM +0900, Michael Paquier wrote:

Thanks for looking, Andres! I guess I'll just apply that once v18
opens up.

Applied as of b19db55bd687.
--
Michael