[Pgpool-hackers] removing backend_socket_dir parameter ?

Fri Jan 7 09:44:30 UTC 2011

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

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 ?

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. 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"

> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese: http://www.sraoss.co.jp

- -- 
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/

iEYEARECAAYFAk0m4HwACgkQXu9L1HbaT6LbXgCg6yFmG8zxBSjuJDV48p0my9gb
7bMAoNwck39grR9I4wsipGnv84GoEEMk
=QHF1
-----END PGP SIGNATURE-----