-----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-----
drop-backend_socket_dir-r3.patch.gz
Description: GNU Zip compressed data
_______________________________________________ Pgpool-hackers mailing list [email protected] http://pgfoundry.org/mailman/listinfo/pgpool-hackers
