> 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.
Looks good. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp _______________________________________________ Pgpool-hackers mailing list [email protected] http://pgfoundry.org/mailman/listinfo/pgpool-hackers
