Remove unused 'len' from pg_stat_recv_* functions

At Tue, 3 Aug 2021 12:40:23 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in

Hi all,

While working on a patch adding new stats, houzj pointed out that
'len' function arguments of all pgstat_recv_* functions are not used
at all. Looking at git history, pgstat_recv_* functions have been
having ‘len’ since the stats collector was introduced by commit
140ddb78fe 20 years ago but it was not used at all even in the first
commit. It seems like the improvements so far for the stats collector
had pgstat_recv_* function have ‘len’ for consistency with the
existing pgstat_recv_* functions. Is there any historical reason for
having 'len' argument? Or can we remove them?

I've attached the patch that removes 'len' from all pgstat_recv_* functions.

I at the first look thought that giving "len" as a parameter is
reasonable as message-processing functions, but the given message
struct contains the same value and the functions can refer to the
message length without the parameter if they want. So I'm +-0 for the
removal.

It applies cleanly on the master and compiled without an error.

That being said, I'm not sure it is worthwhile to change parameters of
going-to-be-removed functions (if shared-memory stats collector is
successfully introduced).

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Tue, Aug 3, 2021 at 3:17 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

At Tue, 3 Aug 2021 12:40:23 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in

Hi all,

While working on a patch adding new stats, houzj pointed out that
'len' function arguments of all pgstat_recv_* functions are not used
at all. Looking at git history, pgstat_recv_* functions have been
having ‘len’ since the stats collector was introduced by commit
140ddb78fe 20 years ago but it was not used at all even in the first
commit. It seems like the improvements so far for the stats collector
had pgstat_recv_* function have ‘len’ for consistency with the
existing pgstat_recv_* functions. Is there any historical reason for
having 'len' argument? Or can we remove them?

I've attached the patch that removes 'len' from all pgstat_recv_* functions.

I at the first look thought that giving "len" as a parameter is
reasonable as message-processing functions, but the given message
struct contains the same value and the functions can refer to the
message length without the parameter if they want. So I'm +-0 for the
removal.

It applies cleanly on the master and compiled without an error.

That being said, I'm not sure it is worthwhile to change parameters of
going-to-be-removed functions (if shared-memory stats collector is
successfully introduced).

Good point.

I'm not going to insist on removing them but I got confused a bit
whether or not I should add 'len' when writing a new pgstat_recv_*
function.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

On 2021-08-03 16:56:12 +0900, Masahiko Sawada wrote:

On Tue, Aug 3, 2021 at 3:17 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:

At Tue, 3 Aug 2021 12:40:23 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in

Hi all,

While working on a patch adding new stats, houzj pointed out that
'len' function arguments of all pgstat_recv_* functions are not used
at all. Looking at git history, pgstat_recv_* functions have been
having ‘len’ since the stats collector was introduced by commit
140ddb78fe 20 years ago but it was not used at all even in the first
commit. It seems like the improvements so far for the stats collector
had pgstat_recv_* function have ‘len’ for consistency with the
existing pgstat_recv_* functions. Is there any historical reason for
having 'len' argument? Or can we remove them?

I've attached the patch that removes 'len' from all pgstat_recv_* functions.

I at the first look thought that giving "len" as a parameter is
reasonable as message-processing functions, but the given message
struct contains the same value and the functions can refer to the
message length without the parameter if they want. So I'm +-0 for the
removal.

It applies cleanly on the master and compiled without an error.

That being said, I'm not sure it is worthwhile to change parameters of
going-to-be-removed functions (if shared-memory stats collector is
successfully introduced).

Good point.

Indeed. It's already kinda painful to maintain the shared memory stats patch,
no need to make it harder...

I'm not going to insist on removing them but I got confused a bit
whether or not I should add 'len' when writing a new pgstat_recv_*
function.

I'd keep it in sync with what's there right now.

Greetings,

Andres Freund