Thread: [HACKERS] pg_basebackup -R
I just tried out pg_basebackup -R and got the following recovery.conf file:
standby_mode = 'on'
primary_conninfo = 'user=rhaas passfile=''/home/rhaas/.pgpass''
port=5432 sslmode=disable sslcompression=1 target_session_attrs=any'
This seems fairly random to me. pg_basebackup explains:
* Do not emit this setting if: - the setting is "replication", * "dbname" or
"fallback_application_name",since these would be * overridden by the libpqwalreceiver module anyway. -
not set or * empty.
...which looks like it got clubbed by pgindent, but anyway I'm not
sure that's the best algorithm. passfile and target_session_attrs are
both new in v10 and have non-empty default values, yet neither of them
seems like something that you necessarily want in your
automatically-created recovery.conf file. It seems like it would be
more correct to leave out parameters that are set to the default
value, rather than those that are set to an empty value.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Feb 8, 2017 at 11:38 PM, Robert Haas <robertmhaas@gmail.com> wrote: > I just tried out pg_basebackup -R and got the following recovery.conf file: > > standby_mode = 'on' > primary_conninfo = 'user=rhaas passfile=''/home/rhaas/.pgpass'' > port=5432 sslmode=disable sslcompression=1 target_session_attrs=any' > > This seems fairly random to me. pg_basebackup explains: > > * Do not emit this setting if: - the setting is "replication", > * "dbname" or "fallback_application_name", since these would be > * overridden by the libpqwalreceiver module anyway. - > not set or > * empty. > > ...which looks like it got clubbed by pgindent, but anyway I'm not > sure that's the best algorithm. passfile and target_session_attrs are > both new in v10 and have non-empty default values, yet neither of them > seems like something that you necessarily want in your > automatically-created recovery.conf file. It seems like it would be > more correct to leave out parameters that are set to the default > value, rather than those that are set to an empty value. > +1. Sounds sensible thing to do. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Thu, Feb 9, 2017 at 8:17 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > +1. Sounds sensible thing to do. Hm. It seems to me that PGPASSFILE still needs to be treated as an exception because it is set to $HOME/.pgpass without any value set in PQconninfoOption->compiled and it depends on the environment. Similar rules apply to fallback_application_name, dbname and replication as well, so they would need to be kept as checked on an individual basis. Now it is true that pg_basebackup -R enforces the value set for a parameter in the created string if its environment variable is set. Bypassing those values would potentially break applications that rely on the existing behavior. In short, I'd like to think that we should just filter out those two parameters by name and call it a day. Or introduce an idea of value set for the environment by adding some kind of tracking flag in PQconninfoOption? Though I am not sure that it is worth complicating libpq to just generate recovery.conf in pg_basebackup. -- Michael
On Thu, Feb 9, 2017 at 9:40 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Thu, Feb 9, 2017 at 8:17 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: >> +1. Sounds sensible thing to do. > > Hm. It seems to me that PGPASSFILE still needs to be treated as an > exception because it is set to $HOME/.pgpass without any value set in > PQconninfoOption->compiled and it depends on the environment. Similar > rules apply to fallback_application_name, dbname and replication as > well, so they would need to be kept as checked on an individual basis. > > Now it is true that pg_basebackup -R enforces the value set for a > parameter in the created string if its environment variable is set. > Bypassing those values would potentially break applications that rely > on the existing behavior. Hmm, I didn't think about environment variables. > In short, I'd like to think that we should just filter out those two > parameters by name and call it a day. Or introduce an idea of value > set for the environment by adding some kind of tracking flag in > PQconninfoOption? Though I am not sure that it is worth complicating > libpq to just generate recovery.conf in pg_basebackup. Yeah, I'm not sure what the best solution is. I just thought it was strange. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Feb 15, 2017 at 2:38 AM, Robert Haas <robertmhaas@gmail.com> wrote: > On Thu, Feb 9, 2017 at 9:40 PM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> In short, I'd like to think that we should just filter out those two >> parameters by name and call it a day. Or introduce an idea of value >> set for the environment by adding some kind of tracking flag in >> PQconninfoOption? Though I am not sure that it is worth complicating >> libpq to just generate recovery.conf in pg_basebackup. > > Yeah, I'm not sure what the best solution is. I just thought it was strange. Thinking more about this, perhaps the correct answer is to do nothing? target_session_attrs being set is rather similar to sslmode or sslcompression for example. They are here but don't hurt. The same thing applies to passfile: if the file is not here the client would still ask for input. If it is here, things are helped a bit. -- Michael
On Tue, Feb 14, 2017 at 11:36 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Wed, Feb 15, 2017 at 2:38 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Thu, Feb 9, 2017 at 9:40 PM, Michael Paquier >> <michael.paquier@gmail.com> wrote: >>> In short, I'd like to think that we should just filter out those two >>> parameters by name and call it a day. Or introduce an idea of value >>> set for the environment by adding some kind of tracking flag in >>> PQconninfoOption? Though I am not sure that it is worth complicating >>> libpq to just generate recovery.conf in pg_basebackup. >> >> Yeah, I'm not sure what the best solution is. I just thought it was strange. > > Thinking more about this, perhaps the correct answer is to do nothing? > target_session_attrs being set is rather similar to sslmode or > sslcompression for example. They are here but don't hurt. The same > thing applies to passfile: if the file is not here the client would > still ask for input. If it is here, things are helped a bit. Let's wait and see if anybody else has an opinion. I imagine that, as further libpq parameters are added, eventually this is going to get long and annoying enough that we want to do something about it. But we might not have reached that point just yet. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2/15/17 10:52, Robert Haas wrote: > Let's wait and see if anybody else has an opinion. I imagine that, as > further libpq parameters are added, eventually this is going to get > long and annoying enough that we want to do something about it. But > we might not have reached that point just yet. I was also annoyed by it when I first saw it. I know why we have to do it, to override any libpq environment variables. Now that I think about it some more, I think we should unset or somehow make libpq ignore all libpq environment variables in the server environment. Nothing good can come of using them. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services