Re: Wrong defeinition of pq_putmessage_noblock since 9.5

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: michael(dot)paquier(at)gmail(dot)com
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, masao(dot)fujii(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, robertmhaas(at)gmail(dot)com
Subject: Re: Wrong defeinition of pq_putmessage_noblock since 9.5
Date: 2016-07-29 05:55:12
Message-ID: 20160729.145512.44798004.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 29 Jul 2016 12:47:53 +0900, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote in <CAB7nPqSSNTroRi=zGMDxYa7PzX_VSck6hbHY6eTnBBsfYaah6A(at)mail(dot)gmail(dot)com>
> On Fri, Jul 29, 2016 at 12:18 PM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> > At Thu, 28 Jul 2016 10:46:00 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <4313(dot)1469717160(at)sss(dot)pgh(dot)pa(dot)us>
> >> Fujii Masao <masao(dot)fujii(at)gmail(dot)com> writes:
> >> > 3. Several source comments in pqcomm.c have not been updated.
> >> > Some comments still use the old function name like pq_putmessage().
> >>
> >> > Attached patch fixes the above issues.
> >>
> >> I dunno, this seems like it's doubling down on some extremely poor
> >> decisions. Why is it that you now have to flip a coin to guess whether
> >> the prefix is pq_ or socket_ for functions in this module? I would
> >> rather see that renaming reverted.
>
> Yes, I agree with that. I cannot understand the intention behind
> 2bd9e41 to rename those routines as they are now, so getting them back
> with pg_ as prefix looks like a good idea to me.

The many of the socket_* functions are required to be replaced
with mq_* functions for backgroud workers. So reverting the names
of socket_* functions should be accompanied by the need to, for
example, changing their callers to use them explicitly via
PqCommMethods, not hiding with macros, or renaming current pq_*
macros to, say, pqi_. (it stands for PQ-Indirect as a tentative)
I'm not confident on the new prefix since I'm not sure that what
the 'pq' stands for. (Postgres Query?)

Attached patch is a rush work to revert the names of socket_
functions and replace the prefix of the macros with "pqi". pq_
names no longer points to mq_ functions. Is this roughly on the
right way? Even though the prefix is not appropriate.

> > The set of functions in PQcommMethods doesn't seem clean. They
> > are chosen arbitrarily just so that other pq_* functions used in
> > parallel workers will work as expected. I suppose that it needs
> > some refactoring.
>
> Any work in this area is likely 10.0 material at this point.
>
> > By the way, pq_start/endcopyout() are used only in FE protocols
> > below 3.0, which had already bacome obsolete as of PG7.4. While
> > the next dev cycle is for PG10, if there is no particular reason
> > to support such ancient protocols, removing them would make things
> > easier and cleaner.
>
> Remove support for protocol 2 has been in the air for some time, but
> that's a separate discussion. If you want to discuss this issue
> particularly, raising a new thread would be a good idea.

Thanks. I saw in somewhere that the protocol 2 no longer
works. I'll raise a new thread later.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-Revert-socket_-functions-name-to-pq_-names.patch text/x-patch 23.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haribabu Kommi 2016-07-29 05:56:30 System load consideration before spawning parallel workers
Previous Message Thomas Munro 2016-07-29 05:39:48 Double invocation of InitPostmasterChild in bgworker with -DEXEC_BACKEND