Re: Remove unused 'len' from pg_stat_recv_* functions

From: Andres Freund <andres(at)anarazel(dot)de>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, houzj(dot)fnst(at)fujitsu(dot)com
Subject: Re: Remove unused 'len' from pg_stat_recv_* functions
Date: 2021-08-03 18:27:40
Message-ID: 20210803182740.qwlkk5vtskm2355g@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-08-03 16:56:12 +0900, Masahiko Sawada wrote:
> On Tue, Aug 3, 2021 at 3:17 PM Kyotaro Horiguchi
> <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> >
> > At Tue, 3 Aug 2021 12:40:23 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2021-08-03 18:30:38 Re: Commitfest overflow
Previous Message Robert Haas 2021-08-03 18:26:16 Re: A varint implementation for PG?