Re: [HACKERS] Passing connection string to pg_basebackup
On Tue, Jan 22, 2013 3:27 PM Hari Babu wrote: >On Saturday, January 19, 2013 5:49 PM Magnus Hagander wrote: >>On Fri, Jan 18, 2013 at 1:05 PM, Heikki Linnakangas >> wrote: >>> On 18.01.2013 13:41, Amit Kapila wrote: On Friday, January 18, 2013 3:46 PM Heikki Linnakangas wrote: > > On 18.01.2013 08:50, Amit Kapila wrote: So to solve this problem below can be done: 1. Support connection string in pg_basebackup and mention keepalives or connection_timeout 2. Support recv_timeout separately to provide a way to users who are not comfortable tcp keepalives a. 1 can be done alone b. 2 can be done alone c. both 1 and 2. >>> >>> >>> Right. Let's do just 1 for now. An general application level, non-TCP, >>> keepalive message at the libpq level might be a good idea, but that's a much >>> larger patch, definitely not 9.3 material. >> >>+1 for doing 1 now. But actually, I think we can just keep it that way >>in the future as well. If you need to specify these fairly advanced >>options, using a connection string really isn't a problem. >> >>I think it would be more worthwhile to go through the rest of the >>tools in bin/ and make sure they *all* support connection strings. >>And, an important point, do it the same way. > >Presently I am trying to implement the option-1 by adding an extra command line >Option -C "connection_string" to pg_basebackup and pg_receivexlog. >This option can be used with all the tools in bin folder. > >The existing command line options to the tools are not planned to remove as of now. > >To handle both options, we can follow these approaches. > >1. To make the code simpler, the connection string is formed inside with the existing >command line options, if the user is not provided the "connection_string" option. >which is used for further processing. > >2. The connection_string and existing command line options are handled separately. > >I feel approach-1 is better. Please provide your suggestions on the same. Here is the patch which handles taking of connection string as an argument to pg_basebackup and pg_receivexlog. Description of changes: 1. New command line "-C connection-string"option is added for passing the connection string. 2. Used "PQconnectdb" function for connecting to server instead of existing function "PQconnectdbParams". 3. The existing command line parameters are formed in a string and passed to "PQconnectdb" function. 4. With the connection string, if user provides additional options with existing command line options, higher priority is given for the additional options. 5. "conninfo_parse" function is modified to handle of single quote in the password provided as input. please provide your suggestions. Regards, Hari babu. pg_basebkup_recvxlog_conn_string_v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On Saturday, January 19, 2013 5:49 PM Magnus Hagander wrote: >On Fri, Jan 18, 2013 at 1:05 PM, Heikki Linnakangas > wrote: >> On 18.01.2013 13:41, Amit Kapila wrote: >>> >>> On Friday, January 18, 2013 3:46 PM Heikki Linnakangas wrote: On 18.01.2013 08:50, Amit Kapila wrote: >>> So to solve this problem below can be done: >>> 1. Support connection string in pg_basebackup and mention keepalives or >>> connection_timeout >>> 2. Support recv_timeout separately to provide a way to users who are not >>> comfortable tcp keepalives >>> >>> a. 1 can be done alone >>> b. 2 can be done alone >>> c. both 1 and 2. >> >> >> Right. Let's do just 1 for now. An general application level, non-TCP, >> keepalive message at the libpq level might be a good idea, but that's a much >> larger patch, definitely not 9.3 material. > >+1 for doing 1 now. But actually, I think we can just keep it that way >in the future as well. If you need to specify these fairly advanced >options, using a connection string really isn't a problem. > >I think it would be more worthwhile to go through the rest of the >tools in bin/ and make sure they *all* support connection strings. >And, an important point, do it the same way. Presently I am trying to implement the option-1 by adding an extra command line Option -C "connection_string" to pg_basebackup and pg_receivexlog. This option can be used with all the tools in bin folder. The existing command line options to the tools are not planned to remove as of now. To handle both options, we can follow these approaches. 1. To make the code simpler, the connection string is formed inside with the existing command line options, if the user is not provided the "connection_string" option. which is used for further processing. 2. The connection_string and existing command line options are handled separately. I feel approach-1 is better. Please provide your suggestions on the same. Regards, Hari babu. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
Robert Haas wrote: > I heartily agree. I can say from firsthand experience that when minor > releases break things for customers (and they do), the customers get > *really* cranky. Based on recent experience, I think we should be > tightening our standards for what gets back-patched, not loosening > them. +1 Any change in a minor release which causes working production code to break very quickly and seriously erodes confidence in the ability to apply a minor release without extensive (and expensive) testing. When that confidence erordes, users stay on old minor releases for extended periods -- often until they hit one of the bugs which was fixed in a minor release. We need to be very conservative about back-patching any changes in user-visible behavior. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On Sat, Jan 19, 2013 at 12:33 PM, Tom Lane wrote: > Dimitri Fontaine writes: >> On the other hand, discrepancies in between command line arguments >> processing in our tools are already not helping our users (even if >> pg_dump -d seems to have been fixed along the years); so much so that >> I'm having a hard time finding any upside into having a different set of >> command line argument capabilities for the same tool depending on the >> major version. > >> We are not talking about a new feature per se, but exposing a feature >> that about every other command line tool we ship have. So I think I'm >> standing on my position that it should get backpatched as a "fix". > > I don't think that argument holds any water at all. There would still > be differences in command line argument capabilities out there --- > they'd just be between minor versions not major ones. That's not any > easier for people to deal with. And what will you say to someone whose > application got broken by a minor-version update? I heartily agree. I can say from firsthand experience that when minor releases break things for customers (and they do), the customers get *really* cranky. Based on recent experience, I think we should be tightening our standards for what gets back-patched, not loosening them. (No, I don't have a specific example off-hand, sorry.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
Tom Lane writes: > I don't think that argument holds any water at all. There would still > be differences in command line argument capabilities out there --- > they'd just be between minor versions not major ones. That's not any > easier for people to deal with. And what will you say to someone whose > application got broken by a minor-version update? Fair enough, I suppose. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
Dimitri Fontaine writes: > On the other hand, discrepancies in between command line arguments > processing in our tools are already not helping our users (even if > pg_dump -d seems to have been fixed along the years); so much so that > I'm having a hard time finding any upside into having a different set of > command line argument capabilities for the same tool depending on the > major version. > We are not talking about a new feature per se, but exposing a feature > that about every other command line tool we ship have. So I think I'm > standing on my position that it should get backpatched as a "fix". I don't think that argument holds any water at all. There would still be differences in command line argument capabilities out there --- they'd just be between minor versions not major ones. That's not any easier for people to deal with. And what will you say to someone whose application got broken by a minor-version update? If this feature were all that critical someone would have noticed its lack before now, anyway. So I can't get excited about back-patching. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
Magnus Hagander writes: >> FWIW, +1. I would consider it a bugfix (backpatch, etc). > > While it's a feature I'd very much like to see, I really don't think > you can consider it a bugfix. It's functionality that was left out - > it's not like we tried to implement it and it didn't work. We pushed > the whole implementation to "next version" (and then forgot about > actually putting it in the next version, until now) Thanks for reminding me about that, I completely forgot about all that. On the other hand, discrepancies in between command line arguments processing in our tools are already not helping our users (even if pg_dump -d seems to have been fixed along the years); so much so that I'm having a hard time finding any upside into having a different set of command line argument capabilities for the same tool depending on the major version. We are not talking about a new feature per se, but exposing a feature that about every other command line tool we ship have. So I think I'm standing on my position that it should get backpatched as a "fix". Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On Fri, Jan 18, 2013 at 2:43 PM, Dimitri Fontaine wrote: > Heikki Linnakangas writes: >> You could still use environment variables and a service file to do it, but >> it's certainly more cumbersome. It clearly should be possible to pass a full >> connection string to pg_basebackup, that's an obvious oversight. > > FWIW, +1. I would consider it a bugfix (backpatch, etc). While it's a feature I'd very much like to see, I really don't think you can consider it a bugfix. It's functionality that was left out - it's not like we tried to implement it and it didn't work. We pushed the whole implementation to "next version" (and then forgot about actually putting it in the next version, until now) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On Fri, Jan 18, 2013 at 1:05 PM, Heikki Linnakangas wrote: > On 18.01.2013 13:41, Amit Kapila wrote: >> >> On Friday, January 18, 2013 3:46 PM Heikki Linnakangas wrote: >>> >>> On 18.01.2013 08:50, Amit Kapila wrote: I think currently user has no way to specify TCP keepalive settings >>> >>> from pg_basebackup, please let me know if there is any such existing way? >>> >>> >>> I was going to say you can just use "keepalives_idle=30" in the >>> connection string. But there's no way to pass a connection string to >>> pg_basebackup on the command line! The usual way to pass a connection >>> string is to pass it as the database name, and PQconnect will expand >>> it, >>> but that doesn't work with pg_basebackup because it hardcodes the >>> database name as "replication". D'oh. >>> >>> You could still use environment variables and a service file to do it, >>> but it's certainly more cumbersome. It clearly should be possible to >>> pass a full connection string to pg_basebackup, that's an obvious >>> oversight. >> >> >> So to solve this problem below can be done: >> 1. Support connection string in pg_basebackup and mention keepalives or >> connection_timeout >> 2. Support recv_timeout separately to provide a way to users who are not >> comfortable tcp keepalives >> >> a. 1 can be done alone >> b. 2 can be done alone >> c. both 1 and 2. > > > Right. Let's do just 1 for now. An general application level, non-TCP, > keepalive message at the libpq level might be a good idea, but that's a much > larger patch, definitely not 9.3 material. +1 for doing 1 now. But actually, I think we can just keep it that way in the future as well. If you need to specify these fairly advanced options, using a connection string really isn't a problem. I think it would be more worthwhile to go through the rest of the tools in bin/ and make sure they *all* support connection strings. And, an important point, do it the same way. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
Heikki Linnakangas writes: > You could still use environment variables and a service file to do it, but > it's certainly more cumbersome. It clearly should be possible to pass a full > connection string to pg_basebackup, that's an obvious oversight. FWIW, +1. I would consider it a bugfix (backpatch, etc). Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On Friday, January 18, 2013 5:35 PM Heikki Linnakangas wrote: > On 18.01.2013 13:41, Amit Kapila wrote: > > On Friday, January 18, 2013 3:46 PM Heikki Linnakangas wrote: > >> On 18.01.2013 08:50, Amit Kapila wrote: > >>> I think currently user has no way to specify TCP keepalive settings > >> from > >>> pg_basebackup, please let me know if there is any such existing > way? > >> > >> I was going to say you can just use "keepalives_idle=30" in the > >> connection string. But there's no way to pass a connection string to > >> pg_basebackup on the command line! The usual way to pass a > connection > >> string is to pass it as the database name, and PQconnect will expand > >> it, > >> but that doesn't work with pg_basebackup because it hardcodes the > >> database name as "replication". D'oh. > >> > >> You could still use environment variables and a service file to do > it, > >> but it's certainly more cumbersome. It clearly should be possible to > >> pass a full connection string to pg_basebackup, that's an obvious > >> oversight. > > > > So to solve this problem below can be done: > > 1. Support connection string in pg_basebackup and mention keepalives > or > > connection_timeout > > 2. Support recv_timeout separately to provide a way to users who are > not > > comfortable tcp keepalives > > > > a. 1 can be done alone > > b. 2 can be done alone > > c. both 1 and 2. > > Right. Let's do just 1 for now. I shall fix it as Review comment and update the patch and change the location from CF-3 to current CF. > An general application level, non-TCP, > keepalive message at the libpq level might be a good idea, but that's a > much larger patch, definitely not 9.3 material. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On 18.01.2013 13:41, Amit Kapila wrote: On Friday, January 18, 2013 3:46 PM Heikki Linnakangas wrote: On 18.01.2013 08:50, Amit Kapila wrote: I think currently user has no way to specify TCP keepalive settings from pg_basebackup, please let me know if there is any such existing way? I was going to say you can just use "keepalives_idle=30" in the connection string. But there's no way to pass a connection string to pg_basebackup on the command line! The usual way to pass a connection string is to pass it as the database name, and PQconnect will expand it, but that doesn't work with pg_basebackup because it hardcodes the database name as "replication". D'oh. You could still use environment variables and a service file to do it, but it's certainly more cumbersome. It clearly should be possible to pass a full connection string to pg_basebackup, that's an obvious oversight. So to solve this problem below can be done: 1. Support connection string in pg_basebackup and mention keepalives or connection_timeout 2. Support recv_timeout separately to provide a way to users who are not comfortable tcp keepalives a. 1 can be done alone b. 2 can be done alone c. both 1 and 2. Right. Let's do just 1 for now. An general application level, non-TCP, keepalive message at the libpq level might be a good idea, but that's a much larger patch, definitely not 9.3 material. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Passing connection string to pg_basebackup
On Friday, January 18, 2013 3:46 PM Heikki Linnakangas wrote: > On 18.01.2013 08:50, Amit Kapila wrote: > > I think currently user has no way to specify TCP keepalive settings > from > > pg_basebackup, please let me know if there is any such existing way? > > I was going to say you can just use "keepalives_idle=30" in the > connection string. But there's no way to pass a connection string to > pg_basebackup on the command line! The usual way to pass a connection > string is to pass it as the database name, and PQconnect will expand > it, > but that doesn't work with pg_basebackup because it hardcodes the > database name as "replication". D'oh. > > You could still use environment variables and a service file to do it, > but it's certainly more cumbersome. It clearly should be possible to > pass a full connection string to pg_basebackup, that's an obvious > oversight. So to solve this problem below can be done: 1. Support connection string in pg_basebackup and mention keepalives or connection_timeout 2. Support recv_timeout separately to provide a way to users who are not comfortable tcp keepalives a. 1 can be done alone b. 2 can be done alone c. both 1 and 2. With Regards, Amit Kapila. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Passing connection string to pg_basebackup
On 18.01.2013 08:50, Amit Kapila wrote: I think currently user has no way to specify TCP keepalive settings from pg_basebackup, please let me know if there is any such existing way? I was going to say you can just use "keepalives_idle=30" in the connection string. But there's no way to pass a connection string to pg_basebackup on the command line! The usual way to pass a connection string is to pass it as the database name, and PQconnect will expand it, but that doesn't work with pg_basebackup because it hardcodes the database name as "replication". D'oh. You could still use environment variables and a service file to do it, but it's certainly more cumbersome. It clearly should be possible to pass a full connection string to pg_basebackup, that's an obvious oversight. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
