[Pgpool-hackers] removing backend_socket_dir parameter ?

Fri Jan 7 10:49:03 UTC 2011

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Le 07/01/2011 11:01, Guillaume Lelarge a écrit :
> Le 07/01/2011 10:59, Tatsuo Ishii a écrit :
>>> Le 07/01/2011 01:55, Tatsuo Ishii a écrit :
>>>>> Le 06/01/2011 15:43, Jehan-Guillaume (ioguix) de Rorthais a écrit :
>>>>>> Le 05/01/2011 10:51, Jehan-Guillaume (ioguix) de Rorthais a écrit :
>>>>>>> Le 05/01/2011 10:22, Tatsuo Ishii a écrit :
>>>>>>>>> Le 04/01/2011 15:38, Jehan-Guillaume (ioguix) de Rorthais a écrit :
>>>>>>>>>> Le 04/01/2011 15:13, Guillaume Lelarge a écrit :
>>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>>> Le 04/01/2011 14:27, Jehan-Guillaume (ioguix) de Rorthais a écrit :
>>>>>>>>>>>> [...]
>>>>>>>>>>>> pgPool is currently using the parameter "backend_socket_dir" to define where it
>>>>>>>>>>>> can find the backend unix socket files.
>>>>>>>>>>>>
>>>>>>>>>>>> However, from the libpq world, we just use one parameter for both unix or inet
>>>>>>>>>>>> socket: host. If this parameter starts with '/', then this is a unix socket. All
>>>>>>>>>>>> other values are inet connections. See:
>>>>>>>>>>>>
>>>>>>>>>>>> http://www.postgresql.org/docs/9.0/static/libpq-connect.html#LIBPQ-CONNECT-HOST
>>>>>>>>>>>>
>>>>>>>>>>>> Back in pgPool world, the equivalent parameter is "backend_hostnameN". So, when
>>>>>>>>>>>> I was setting up a dev environment yesterday, I naturally set this parameter  to
>>>>>>>>>>>> my unix path (I'm using Debian) and end up with a hostname resolution error.
>>>>>>>>>>>>
>>>>>>>>>>>> So you'll find in attachment a patch to remove the "backend_socket_dir"
>>>>>>>>>>>> parameter and use the libpq policy. Moreover, on empty "backend_hostnameN"
>>>>>>>>>>>> value, the patch fall back to DEFAULT_SOCKET_DIR.
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> This patch is really interesting. This is something I had on my todo
>>>>>>>>>>> list for quite some time.
>>>>>>>>>>
>>>>>>>>>> Glad to hear that :)
>>>>>>>>>> I wasn't the only one wondering about this then :)
>>>>>>>>>>
>>>>>>>>>>>> I realize it will break compatibility with older configuration file. But in a
>>>>>>>>>>>> first step, I prefer something clean and start discussing this issue with you if
>>>>>>>>>>>> you really mind this.
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On the configuration file, compatibility of configuration file is never
>>>>>>>>>>> garantied. So I don't think this is such an issue. We need to make it
>>>>>>>>>>> clear that the parameter has a new mail.
>>>>>>>>>>
>>>>>>>>>> s/new mail/new name/
>>>>>>>>>>
>>>>>>>>>> well, it's not a new name, we just drop one and promote an existing one to deal
>>>>>>>>>> with both unix and inet socket :)
>>>>>>>>>>
>>>>>>>>>>> If we really want to maintain the compatibility on this issue, we could
>>>>>>>>>>> still support the old parameter during two or three major releases. I
>>>>>>>>>>> guess we need to put a warning in the log to say "hey guy, you're still
>>>>>>>>>>> using that old obsolete configuration parameter, you should better
>>>>>>>>>>> change with this one".
>>>>>>>>>>
>>>>>>>>>> Yeah. What I have in mind is :
>>>>>>>>>>   if backend_hostnameN == ''
>>>>>>>>>>     if not backend_socket_dir
>>>>>>>>>>       backend_hostnameN = DEFAULT_SOCKET_DIR
>>>>>>>>>>     else
>>>>>>>>>>       log « please stop using that »
>>>>>>>>>>       backend_hostnameN = backend_socket_dir
>>>>>>>>>>
>>>>>>>>>> But really, I would prefer to drop backend_socket_dir and keep the code clean.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> You can't have it both ways. Either you please your users, or you please
>>>>>>>>> your developpers. I think we should do the former.
>>>>>>
>>>>>>>> Agreed. I learned it in a hard way:-)
>>>>>>
>>>>>>>> Now I think the idea is great and am looking forward to accepting for
>>>>>>>> the next release.
>>>>>>
>>>>>>> Thank you !
>>>>>>
>>>>>> So here is the new patch including backward compatibility with backend_socket_dir.
>>>>>>
>>>>>
>>>>> Seems good to me. Could you also get rid of backend_socket_dir in the
>>>>> sample configuration files? this parameter won't be the prefered way to
>>>>> set the socket, so it shouldn't be in the sample conf files.
>>>>
>>>> But according to the patches still backend_socket_dir remains and even
>>>> appears on show pool_status.
>>>
>>> Yes, I kept it because cause you agree'd Guillaume statement:
>>> «
>>> You can't have it both ways. Either you please your users, or you please your
>>> developpers. I think we should do the former.
>>> »
>>>
>>> My anderstanding is that "doing the former", so "please your users", means
>>> keeping backward compatibility for some time. Are we ok ?
>>
>> Yup.
>>
>>> So my last patch implement the logic I described above:
>>>
>>>   if (backend_hostnameN == '' and backend_socket_dir != '')
>>>     log "this parameter is deprecated ! use backend_socket_dir instead."
>>>     backend_hostnameN = backend_socket_dir
>>>
>>>
>>> So yes, it appears in show pool_status :(
>>>
>>>> Probably we should leave it in the
>>>> pgpool.conf.sample etc. with statting that "this directive is
>>>> obsoleted and used for only internal use." or something like that?
>>>
>>> We don't use it internally.
>>
>> Oh ok.
>>
>>> In my opinion, we should
>>>   * remove it from the sample
>>>   * warn the user if he still use it in its configuration file
>>>   * add "DEPCRECATED use backend_hostname instead" as description for this
>>> parameter in "show pool_status"
>>
>> If you and Guillaume agreed on this, I'm fine with removing it from
>> the sample conf.
> 
> Yeah, I think with these three items.

So here is the patch.

I added a check in pool_process_reporting to hide backend_socket_dir if it is
not set in the conf file.

I'm waiting for the feedback to patch the doc.
- -- 
Jehan-Guillaume de Rorthais
DBA
http://www.dalibo.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk0m754ACgkQXu9L1HbaT6LpHACdGWg5CrWbXYFDr2QWw223FPMm
JBEAn3c6Eo7zjj/dhKPHG9BwWrFyxuym
=SUNB
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: drop-backend_socket_dir-r3.patch.gz
Type: application/x-gzip
Size: 1640 bytes
Desc: not available
URL: <http://pgfoundry.org/pipermail/pgpool-hackers/attachments/20110107/0a686d64/attachment.bin>