Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Nov 19, 2013 at 2:06 PM, Haribabu kommi wrote: > On 19 November 2013 09:59 Amit Kapila wrote: >> On Mon, Nov 18, 2013 at 8:31 PM, Haribabu kommi >> wrote: >> > On 18 November 2013 20:01 Amit Kapila wrote: >> >> > Code changes are fine. >> >> > If two or three errors are present in the configuration file, it >> >> prints the last error > > Ok fine I marked the patch as ready for committer. Thanks for review. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 19 November 2013 09:59 Amit Kapila wrote: > On Mon, Nov 18, 2013 at 8:31 PM, Haribabu kommi > wrote: > > On 18 November 2013 20:01 Amit Kapila wrote: > >> > Code changes are fine. > >> > If two or three errors are present in the configuration file, it > >> prints the last error > >> > Configuration parameter file only. Is it required to be mentioned > >> > in > >> the documentation? > >> > >> Do you mean to say parsing errors or some run-time error, could you > >> explain with example? > > > > LOG: parameter "shared_buffers" cannot be changed without restarting > > the server > > LOG: parameter "port" cannot be changed without restarting the > server > > LOG: configuration file > > "/home/hari/installation/bin/../../data/postgresql.auto.conf" > contains > > errors; unaffected changes were applied > > > > The shared_buffers parameter is changed in postgresql.conf and port > is changed in postgresql.auto.conf. > > The error file displays the last error occurred file. > >This is only possible if user tries to change configuration > parameters both by Alter System command and manually as well and the > case above is >not an error, it is just an information for user that there are some > parameters changed in config file which can only get reflected after > server restart. >So definitely user can check log files to see which parameter's > doesn't get reflected if he is expecting some parameter to be changed, > but it's not >changed. I think here even in logs if last file containing errors is > mentioned is not a big problem. > > > Is it required to mention the above behavior in the document? > >It is better to show in log both the files rather than documenting > it, if the the above case is helpful for user which I don't think so, > also to handle case >in code can complicate the error handling path of code a bit. So I > think we can leave this case as-is. Ok fine I marked the patch as ready for committer. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Mon, Nov 18, 2013 at 8:31 PM, Haribabu kommi wrote: > On 18 November 2013 20:01 Amit Kapila wrote: >> > Code changes are fine. >> > If two or three errors are present in the configuration file, it >> prints the last error >> > Configuration parameter file only. Is it required to be mentioned in >> the documentation? >> >> Do you mean to say parsing errors or some run-time error, could you >> explain with example? > > LOG: parameter "shared_buffers" cannot be changed without restarting the > server > LOG: parameter "port" cannot be changed without restarting the server > LOG: configuration file > "/home/hari/installation/bin/../../data/postgresql.auto.conf" contains > errors; unaffected changes were applied > > The shared_buffers parameter is changed in postgresql.conf and port is > changed in postgresql.auto.conf. > The error file displays the last error occurred file. This is only possible if user tries to change configuration parameters both by Alter System command and manually as well and the case above is not an error, it is just an information for user that there are some parameters changed in config file which can only get reflected after server restart. So definitely user can check log files to see which parameter's doesn't get reflected if he is expecting some parameter to be changed, but it's not changed. I think here even in logs if last file containing errors is mentioned is not a big problem. > Is it required to mention the above behavior in the document? It is better to show in log both the files rather than documenting it, if the the above case is helpful for user which I don't think so, also to handle case in code can complicate the error handling path of code a bit. So I think we can leave this case as-is. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 18 November 2013 20:01 Amit Kapila wrote: > On Mon, Nov 18, 2013 at 6:28 PM, Haribabu kommi > wrote: > > On 17 November 2013 12:25 Amit Kapila wrote: > >> On Sat, Nov 16, 2013 at 4:35 PM, Haribabu kommi > >> >> >> Find the rebased version attached with this mail. > >> >> > > >> > ereport(ERROR, > >> > > >> (errcode(ERRCODE_CONFIG_FILE_ERROR), > >> > errmsg("configuration > file > >> \"%s\" contains errors", > >> > - > >> ConfigFileName))); > >> > + > >> > + ErrorConfFile))); > >> > > >> > The ErrorConfFile prints "postgresql.auto.conf" only if there is > >> > any parsing problem with postgresql.auto.conf otherwise it always > >> > print > >> "postgresql.conf" because of any other error. > >> > >>Changed to ensure ErrorConfFile contains proper config file name. > >>Note: I have not asssigned file name incase of error in below > >> loop, as file name in gconf is NULL in most cases and moreover this > >> loops over > >> guc_variables which doesn't contain values/parameters > >> from auto.conf. So I don't think it is required to assign > >> ErrorConfFile in this loop. > >> > >> ProcessConfigFile(GucContext context) { .. > >>for (i = 0; i < num_guc_variables; i++) > >>{ > >>struct config_generic *gconf = guc_variables[i]; > >> > >> .. > >> } > > > > Code changes are fine. > > If two or three errors are present in the configuration file, it > prints the last error > > Configuration parameter file only. Is it required to be mentioned in > the documentation? > > Do you mean to say parsing errors or some run-time error, could you > explain with example? LOG: parameter "shared_buffers" cannot be changed without restarting the server LOG: parameter "port" cannot be changed without restarting the server LOG: configuration file "/home/hari/installation/bin/../../data/postgresql.auto.conf" contains errors; unaffected changes were applied The shared_buffers parameter is changed in postgresql.conf and port is changed in postgresql.auto.conf. The error file displays the last error occurred file. Is it required to mention the above behavior in the document? 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Mon, Nov 18, 2013 at 6:28 PM, Haribabu kommi wrote: > On 17 November 2013 12:25 Amit Kapila wrote: >> On Sat, Nov 16, 2013 at 4:35 PM, Haribabu kommi >> >> >> Find the rebased version attached with this mail. >> >> > >> > ereport(ERROR, >> > >> (errcode(ERRCODE_CONFIG_FILE_ERROR), >> > errmsg("configuration file >> \"%s\" contains errors", >> > - >> ConfigFileName))); >> > + >> > + ErrorConfFile))); >> > >> > The ErrorConfFile prints "postgresql.auto.conf" only if there is any >> > parsing problem with postgresql.auto.conf otherwise it always print >> "postgresql.conf" because of any other error. >> >>Changed to ensure ErrorConfFile contains proper config file name. >>Note: I have not asssigned file name incase of error in below loop, >> as file name in gconf is NULL in most cases and moreover this loops >> over >> guc_variables which doesn't contain values/parameters from >> auto.conf. So I don't think it is required to assign ErrorConfFile in >> this loop. >> >> ProcessConfigFile(GucContext context) >> { >> .. >>for (i = 0; i < num_guc_variables; i++) >>{ >>struct config_generic *gconf = guc_variables[i]; >> >> .. >> } > > Code changes are fine. > If two or three errors are present in the configuration file, it prints the > last error > Configuration parameter file only. Is it required to be mentioned in the > documentation? Do you mean to say parsing errors or some run-time error, could you explain with example? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 17 November 2013 12:25 Amit Kapila wrote: > On Sat, Nov 16, 2013 at 4:35 PM, Haribabu kommi > wrote: > > On 16 November 2013 09:43 Amit Kapila wrote: > >> On Fri, Nov 15, 2013 at 10:18 PM, Peter Eisentraut > >> wrote: > >> > On 11/14/13, 2:50 AM, Amit Kapila wrote: > >> >> Find the rebased version attached with this mail. > >> > > > ereport(ERROR, > > > (errcode(ERRCODE_CONFIG_FILE_ERROR), > > errmsg("configuration file > \"%s\" contains errors", > > - > ConfigFileName))); > > + > > + ErrorConfFile))); > > > > The ErrorConfFile prints "postgresql.auto.conf" only if there is any > > parsing problem with postgresql.auto.conf otherwise it always print > "postgresql.conf" because of any other error. > >Changed to ensure ErrorConfFile contains proper config file name. >Note: I have not asssigned file name incase of error in below loop, > as file name in gconf is NULL in most cases and moreover this loops > over > guc_variables which doesn't contain values/parameters from > auto.conf. So I don't think it is required to assign ErrorConfFile in > this loop. > > ProcessConfigFile(GucContext context) > { > .. >for (i = 0; i < num_guc_variables; i++) >{ >struct config_generic *gconf = guc_variables[i]; > > .. > } Code changes are fine. If two or three errors are present in the configuration file, it prints the last error Configuration parameter file only. Is it required to be mentioned in the documentation? > > > > if any postmaster setting which are set by the alter system command > > which leads to failure of server start, what is the solution to user > > to proceed further to start the server. As it is mentioned that the > > auto.conf file shouldn't be edited manually. > > Yeah, but in case of emergency user can change it to get server started. > Now the question is whether to mention it in documentation, I think we > can leave this decision to committer. If he thinks that it is better to > document then I will update it. Ok fine. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Sat, Nov 16, 2013 at 4:35 PM, Haribabu kommi wrote: > On 16 November 2013 09:43 Amit Kapila wrote: >> On Fri, Nov 15, 2013 at 10:18 PM, Peter Eisentraut >> wrote: >> > On 11/14/13, 2:50 AM, Amit Kapila wrote: >> >> Find the rebased version attached with this mail. >> > > Please find review comments: > > + * ALTER SYSTEM SET > + * > + * Command to edit postgresql.conf > + > */ > > I feel it should be "Command to change the configuration parameter" > because this command is not edits the postgresql.conf file. Changed the comment, but I think it is better to use persistently in line suggested by you, so I have done that way. > ereport(ERROR, > (errcode(ERRCODE_CONFIG_FILE_ERROR), > errmsg("configuration file \"%s\" > contains errors", > - ConfigFileName))); > + ErrorConfFile))); > > The ErrorConfFile prints "postgresql.auto.conf" only if there is any parsing > problem > with postgresql.auto.conf otherwise it always print "postgresql.conf" because > of any other error. Changed to ensure ErrorConfFile contains proper config file name. Note: I have not asssigned file name incase of error in below loop, as file name in gconf is NULL in most cases and moreover this loops over guc_variables which doesn't contain values/parameters from auto.conf. So I don't think it is required to assign ErrorConfFile in this loop. ProcessConfigFile(GucContext context) { .. for (i = 0; i < num_guc_variables; i++) { struct config_generic *gconf = guc_variables[i]; .. } > > + * A stale temporary file may be left behind in case we crash. > + * Such files are removed on the next server restart. > > The above comment is wrong, the stale temporary file will be used > in the next ALTER SYSTEM command. I didn't find any code where it gets > deleted on the next server restart. Removed the comment from top of function and modified the comment where file is getting opened. > > if any postmaster setting which are set by the alter system command which > leads to failure of server start, what is the solution to user to proceed > further to start the server. As it is mentioned that the auto.conf file > shouldn't be edited manually. Yeah, but in case of emergency user can change it to get server started. Now the question is whether to mention it in documentation, I think we can leave this decision to committer. If he thinks that it is better to document then I will update it. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com alter_system_v12.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 16 November 2013 09:43 Amit Kapila wrote: > On Fri, Nov 15, 2013 at 10:18 PM, Peter Eisentraut > wrote: > > On 11/14/13, 2:50 AM, Amit Kapila wrote: > >> Find the rebased version attached with this mail. > > > > Doesn't build: > > > > openjade -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D . > -c /usr/share/sgml/docbook/stylesheet/dsssl/modular/catalog -d > stylesheet.dsl -t sgml -i output-html -V html-index postgres.sgml > > openjade:reference.sgml:61:3:E: cannot find "alter_system.sgml"; > tried "ref/alter_system.sgml", "./alter_system.sgml", > "./alter_system.sgml", "/usr/local/share/sgml/alter_system.sgml", > "/usr/share/sgml/alter_system.sgml" > > openjade:config.sgml:164:27:X: reference to non-existent ID "SQL- > ALTERSYSTEM" > > make[3]: *** [HTML.index] Error 1 > > make[3]: *** Deleting file `HTML.index' > > osx -D. -x lower -i include-xslt-index postgres.sgml >postgres.xmltmp > > osx:reference.sgml:61:3:E: cannot find "alter_system.sgml"; tried > "ref/alter_system.sgml", "./alter_system.sgml", > "/usr/local/share/sgml/alter_system.sgml", > "/usr/share/sgml/alter_system.sgml" > > osx:config.sgml:164:27:X: reference to non-existent ID "SQL- > ALTERSYSTEM" > > make[3]: *** [postgres.xml] Error 1 > > > > New file missing in patch? > > Oops, missed the new sgml file in patch, updated patch to include it. > Many thanks for checking it. 1. Patch applied properly 2. No warnings in the compilation 3. Regress test passed. 4. Basic tests are passed. Please find review comments: + * ALTER SYSTEM SET + * + * Command to edit postgresql.conf + */ I feel it should be "Command to change the configuration parameter" because this command is not edits the postgresql.conf file. ereport(ERROR, (errcode(ERRCODE_CONFIG_FILE_ERROR), errmsg("configuration file \"%s\" contains errors", - ConfigFileName))); + ErrorConfFile))); The ErrorConfFile prints "postgresql.auto.conf" only if there is any parsing problem with postgresql.auto.conf otherwise it always print "postgresql.conf" because of any other error. + * A stale temporary file may be left behind in case we crash. + * Such files are removed on the next server restart. The above comment is wrong, the stale temporary file will be used in the next ALTER SYSTEM command. I didn't find any code where it gets deleted on the next server restart. if any postmaster setting which are set by the alter system command which leads to failure of server start, what is the solution to user to proceed further to start the server. As it is mentioned that the auto.conf file shouldn't be edited manually. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Nov 15, 2013 at 10:18 PM, Peter Eisentraut wrote: > On 11/14/13, 2:50 AM, Amit Kapila wrote: >> Find the rebased version attached with this mail. > > Doesn't build: > > openjade -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D . -c > /usr/share/sgml/docbook/stylesheet/dsssl/modular/catalog -d stylesheet.dsl -t > sgml -i output-html -V html-index postgres.sgml > openjade:reference.sgml:61:3:E: cannot find "alter_system.sgml"; tried > "ref/alter_system.sgml", "./alter_system.sgml", "./alter_system.sgml", > "/usr/local/share/sgml/alter_system.sgml", "/usr/share/sgml/alter_system.sgml" > openjade:config.sgml:164:27:X: reference to non-existent ID "SQL-ALTERSYSTEM" > make[3]: *** [HTML.index] Error 1 > make[3]: *** Deleting file `HTML.index' > osx -D. -x lower -i include-xslt-index postgres.sgml >postgres.xmltmp > osx:reference.sgml:61:3:E: cannot find "alter_system.sgml"; tried > "ref/alter_system.sgml", "./alter_system.sgml", > "/usr/local/share/sgml/alter_system.sgml", "/usr/share/sgml/alter_system.sgml" > osx:config.sgml:164:27:X: reference to non-existent ID "SQL-ALTERSYSTEM" > make[3]: *** [postgres.xml] Error 1 > > New file missing in patch? Oops, missed the new sgml file in patch, updated patch to include it. Many thanks for checking it. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com alter_system_v11.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 11/14/13, 2:50 AM, Amit Kapila wrote: > Find the rebased version attached with this mail. Doesn't build: openjade -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D . -c /usr/share/sgml/docbook/stylesheet/dsssl/modular/catalog -d stylesheet.dsl -t sgml -i output-html -V html-index postgres.sgml openjade:reference.sgml:61:3:E: cannot find "alter_system.sgml"; tried "ref/alter_system.sgml", "./alter_system.sgml", "./alter_system.sgml", "/usr/local/share/sgml/alter_system.sgml", "/usr/share/sgml/alter_system.sgml" openjade:config.sgml:164:27:X: reference to non-existent ID "SQL-ALTERSYSTEM" make[3]: *** [HTML.index] Error 1 make[3]: *** Deleting file `HTML.index' osx -D. -x lower -i include-xslt-index postgres.sgml >postgres.xmltmp osx:reference.sgml:61:3:E: cannot find "alter_system.sgml"; tried "ref/alter_system.sgml", "./alter_system.sgml", "/usr/local/share/sgml/alter_system.sgml", "/usr/share/sgml/alter_system.sgml" osx:config.sgml:164:27:X: reference to non-existent ID "SQL-ALTERSYSTEM" make[3]: *** [postgres.xml] Error 1 New file missing in patch? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Nov 13, 2013 at 7:23 PM, Amit Kapila wrote: > On Wed, Nov 13, 2013 at 4:05 PM, Haribabu kommi > wrote: >> On 01 October 2013 00:56 Amit Kapila wrote: >>> On Mon, Sep 30, 2013 at 9:07 PM, Peter Eisentraut >>> wrote: >>> > On 9/28/13 3:05 AM, Amit Kapila wrote: >>> >> Now as we have an agreement, I had updated patch for below left >>> issues: >>> > >> >> Some of the initial observation of the patch are, >> 1. Patch is not applying against git head, needs a rebase. Find the rebased version attached with this mail. I have removed header inclusions of port.h and lwlock.h from guc.c, it works fine without them. >> 2. Patch doesn't contain the tests. >It was intentional and as per feedback for this patch. As for > testing this feature, we need to put sleep after operation, so it was > suggested to remove tests. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com alter_system_v10.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Nov 13, 2013 at 4:05 PM, Haribabu kommi wrote: > On 01 October 2013 00:56 Amit Kapila wrote: >> On Mon, Sep 30, 2013 at 9:07 PM, Peter Eisentraut >> wrote: >> > On 9/28/13 3:05 AM, Amit Kapila wrote: >> >> Now as we have an agreement, I had updated patch for below left >> issues: >> > >> > Regression tests are failing. >> >>Thanks for informing. I am sorry for not running regression before >> sending patch. >> >>Reason for failure was that source for GUC in new function >> validate_conf_option() was hardcoded to PGC_S_FILE which was okay for >> Alter System, but >>not for SET path. I had added new parameter source in this function >> and get the value of source when this is called from SET path. > > Some of the initial observation of the patch are, > 1. Patch is not applying against git head, needs a rebase. > 2. Patch doesn't contain the tests. It was intentional and as per feedback for this patch. As for testing this feature, we need to put sleep after operation, so it was suggested to remove tests. > I started reviewing the patch, will share the details once I finish. Thanks. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 01 October 2013 00:56 Amit Kapila wrote: > On Mon, Sep 30, 2013 at 9:07 PM, Peter Eisentraut > wrote: > > On 9/28/13 3:05 AM, Amit Kapila wrote: > >> Now as we have an agreement, I had updated patch for below left > issues: > > > > Regression tests are failing. > >Thanks for informing. I am sorry for not running regression before > sending patch. > >Reason for failure was that source for GUC in new function > validate_conf_option() was hardcoded to PGC_S_FILE which was okay for > Alter System, but >not for SET path. I had added new parameter source in this function > and get the value of source when this is called from SET path. Some of the initial observation of the patch are, 1. Patch is not applying against git head, needs a rebase. 2. Patch doesn't contain the tests. I started reviewing the patch, will share the details once I finish. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Mon, Sep 30, 2013 at 9:07 PM, Peter Eisentraut wrote: > On 9/28/13 3:05 AM, Amit Kapila wrote: >> Now as we have an agreement, I had updated patch for below left issues: > > Regression tests are failing. Thanks for informing. I am sorry for not running regression before sending patch. Reason for failure was that source for GUC in new function validate_conf_option() was hardcoded to PGC_S_FILE which was okay for Alter System, but not for SET path. I had added new parameter source in this function and get the value of source when this is called from SET path. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com alter_system_v9.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 9/28/13 3:05 AM, Amit Kapila wrote: > Now as we have an agreement, I had updated patch for below left issues: Regression tests are failing. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Aug 30, 2013 at 8:24 PM, Tom Lane wrote: > Stephen Frost writes: >> * Robert Haas (robertmh...@gmail.com) wrote: >>> I think you're getting way too hung up on the fact that the proposed >>> auto.conf will be stored as a flat file. From your comments upthread, >>> I gather that you'd be rejoicing if it were a table. > >> I'd be happy if it was a table which managed an *independent* set of >> parameters from those used to bootstrap the system, but no one seems to >> like breaking up the options between "things that can be sanely modified >> without other OS changes" and "things which require OS support". > > I agree with Robert's comments upthread that if the new facility can't do > everything that can be done today by editing postgresql.conf, it's not > going to be adequate. So I'm not in favor of having two sets of > parameters. It's also not clear to me that we can make a reliable > distinction between parameters that can prevent a server restart vs those > that can't; or at least, the set of the latter will be much smaller than > one could wish. Now as we have an agreement, I had updated patch for below left issues: 1. ALTER SYSTEM SET should be constrained to only set known GUCs, this point has been discussed on another mail thread (http://www.postgresql.org/message-id/14857.1378523...@sss.pgh.pa.us) In function AlterSystemSetConfigFile(), when we try to get the record using find_option(), pass second parameter as false which will make sure if the parameter doesn't exist it will return NULL. 2. Some indentation issues. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com alter_system_v8.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost writes: > * Robert Haas (robertmh...@gmail.com) wrote: >> I think you're getting way too hung up on the fact that the proposed >> auto.conf will be stored as a flat file. From your comments upthread, >> I gather that you'd be rejoicing if it were a table. > I'd be happy if it was a table which managed an *independent* set of > parameters from those used to bootstrap the system, but no one seems to > like breaking up the options between "things that can be sanely modified > without other OS changes" and "things which require OS support". I agree with Robert's comments upthread that if the new facility can't do everything that can be done today by editing postgresql.conf, it's not going to be adequate. So I'm not in favor of having two sets of parameters. It's also not clear to me that we can make a reliable distinction between parameters that can prevent a server restart vs those that can't; or at least, the set of the latter will be much smaller than one could wish. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Robert Haas (robertmh...@gmail.com) wrote: > I think you're getting way too hung up on the fact that the proposed > auto.conf will be stored as a flat file. From your comments upthread, > I gather that you'd be rejoicing if it were a table. I'd be happy if it was a table which managed an *independent* set of parameters from those used to bootstrap the system, but no one seems to like breaking up the options between "things that can be sanely modified without other OS changes" and "things which require OS support". Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Tom, all, I'm not one to give up a fight (I hope that's something ya'll like about me ;), but in this case I'm gonna have to concede. Clearly, I'm in the minority about this, at least on the lists and among the active hackers. Let me just say that I hope all the happy users of this will outweigh the complaints. I suppose there's only one way to find out. Thanks! Stephen * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Robert Haas (robertmh...@gmail.com) wrote: > >> I think you're getting way too hung up on the fact that the proposed > >> auto.conf will be stored as a flat file. From your comments upthread, > >> I gather that you'd be rejoicing if it were a table. > > > I'd be happy if it was a table which managed an *independent* set of > > parameters from those used to bootstrap the system, but no one seems to > > like breaking up the options between "things that can be sanely modified > > without other OS changes" and "things which require OS support". > > I agree with Robert's comments upthread that if the new facility can't do > everything that can be done today by editing postgresql.conf, it's not > going to be adequate. So I'm not in favor of having two sets of > parameters. It's also not clear to me that we can make a reliable > distinction between parameters that can prevent a server restart vs those > that can't; or at least, the set of the latter will be much smaller than > one could wish. > > regards, tom lane signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Aug 30, 2013 at 8:53 AM, Stephen Frost wrote: > > Yes, one of the issues with the existing system is that you can't > > specify a default to be applied to new roles. Also, there are > > parameters which are not per-role yet which it probably makes sense to > > be in the database and we'd need a way to deal with that. Although, at > > the same time, considering it role based does make for a nice > > distinction. Unfortunately, those clammoring for this will argue that > > it wouldn't replace adminpack and that they couldn't use it to modify > > their /etc/network/interfaces file, which is the obvious next step to > > all of this. > > This is a straw-man. It was intended to be and I wasn't particularly expecting a response to the sarcasm, at the same time.. > Likening this to the > feature being proposed is silly. What is being asked for here is the > ability to easily modify system-wide settings from the SQL prompt, > just as today you can modify settings per-user or per-database. /etc/network/interfaces would be considered part of the "system" by some.. :) PG isn't an OS, yet.. > That's not the same thing as rewriting the entire system > configuration. And here you're using the *other* meaning of system, no? Glad there won't be any confusion here! ;) (yes, I'm kidding..). Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Aug 30, 2013 at 9:49 AM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: >> there's a fairly generous but fixed-at-startup-time limit on how many >> segments you can have. In practice I don't think this matters much, >> but it was a sobering reminder that the main shared memory segment, >> with all of its inflexibility, has important reliability properties >> that are hard to replicate in more dynamic scenarios. > > Why wouldn't it be possible to have the same arrangment for > shared_buffers, where you have more entires than you 'need' at startup > but which then allows you to add more shared segments later? I do see > that we would need an additional bit of indirection to handle that, > which might be too much overhead, but the concept seems possible. Isn't > that more-or-less how the kernel handles dynamic memory..? Yeah, I think that something like would be possible, but the synchronization would be tricky, and it wouldn't work at all with System V shared memory or Windows shared memory, which apparently can't be resized after creation. Also, it would rely on the system administrator having a sensible setting for max_on_the_fly_shared_buffers and only having the wrong setting for initial_shared_buffers_until_i_change_it_later, which might not cover a very high percentage of the cases that arise in practice. >> Under the currently-proposed design, it can't be used to do any such >> thing. It can only be used to modify some auto.conf file which lives >> in $PGDATA. It's therefore no different from the ops perspective than >> ALTER DATABASE or ALTER USER - except that it allows all settings to >> be changed rather than only a subset. > > Claiming that modifying a file *included from a file in /etc* doesn't > modify things under /etc is disingenuous, imv. I think you're getting way too hung up on the fact that the proposed auto.conf will be stored as a flat file. From your comments upthread, I gather that you'd be rejoicing if it were a table. The only reason we're not doing that is because of the possibility that something in auto.conf might keep the server from starting. I don't think that's gonna be very common now that we're mostly out from under the System V shared memory limits, but we'll see. As you point out, it might also be worth fixing: maybe we can find some way to arrange things so that the server will always be able to start up regardless of how badly messed-up auto.conf is... but that's a different patch. -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > Technically trivial in the sense that it should be queryable from SQL > without having to write code in an untrusted PL ;). hah. > I guess storing the file modification date along the file/location a GUC > is originating from would be good enough. Then you could write a query > using pg_stat_file() to make sure they are up2date. Wouldn't you want to actually look at the GUC and see if the value is different also? Just knowing that postgresql.conf changed doesn't mean you want every value to say it's different... It's entirely possible that the file was rewritten or touch'd but the configuration is identical. > > > * Configuration variables only come from locations that are approved for > > > in your scenario (Already possible, we might want to make it even > > > easier) > > > > That an interesting notion; do you have something specific in mind..? > > The easiest, imv anyway, would be that options set in postgresql.conf > > can't be overridden, but that gets us into the bootstrap problem that > > people seem to be concerned about. It would also be a change to how > > postgresql.conf is parsed today which some people would be annoyed by. > > Having some configuration option which says what can be modified by > > alter system doesn't strike me as a terribly good solution either. > > I think changing the precedence of options in postgresql.conf has about > zero chance. Sadly, you're probably right. > That would make it possible to easily write a query that works across > intallation that warns about any values stored in auto.conf, even if > they are overwritten by a per-user config or similar. I had the impression that 'approved for' above meant something which actually *enforced* it rather than just another monitoring check.. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Aug 30, 2013 at 9:19 AM, Stephen Frost wrote: > You and Robert both seem to be of the opinion that this hack which > brings postgresql.conf into the database via ALTER SYSTEM is a-ok > because it's moving us "forward" in someone's mind, but it really is > developing a system configuration management system which *looks* like a > first-class citizen when it actually falls woefully short of that. It's not a system configuration management system, and it doesn't pretend to be. It's an analogue of ALTER USER and ALTER DATABASE that papers over their shortcomings, and a safer alternative to adminpack's kludgy way of enabling the same functionality. > There is absolutely no question in my mind that this will be a huge > support pain, from the first "ALTER SYSTEM SET shared_buffers = blah; > SHOW shared_buffers;" They'd have the same problem with ALTER USER SET work_mem = blah. You're acting like ALTER commands that don't take immediate effect are something brand-new, but we've had them for years. > to the "why can't my database start?!? it's > complaining it can't allocate memory but I keep changing postgresql.conf > and nothing works!" As of 9.3, that failure mode doesn't really happen any more, unless you maybe set shared_buffers equal to 100% of system memory. > I'm simply not convinced that this is moving us > forward nor that we will end up with more benefit than pain from it. Fair enough, but I'm not convinced that we'll derive any pain at all from it. The existing similar features haven't been a notable source of complaints AFAIK. -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-30 09:43:01 -0400, Stephen Frost wrote: > * Andres Freund (and...@2ndquadrant.com) wrote: > > On 2013-08-30 09:19:42 -0400, Stephen Frost wrote: > > > wal_level and shared_buffers I can buy, but listen_addresses? The most > > > typical change there is going from localhost -> '*', but you've got to > > > be on the box to do that. Anything else and you're going to need to be > > > adding interfaces to the box anyway and hacking around in > > > /etc/network/interfaces or what-have-you. > > > > Even if it requires to be on the box locally, I only need libpq for > > it. And it's not infrequent to allow additional, already configured > > interfaces. And even if not, what's the point of prohibiting > > listen_interfaces specifically? When the other very interesting > > variables have the same dangers? > > I'd like to see those dangers removed from the other very interesting > variables. We're making progress towards that, for example with > shared_buffers. I've commented on that already up-thread. Hell, you > could even make things such that PG would start w/ a misconfigured > listen_addresses- but if we don't like that then I would argue that it > shouldn't be included here. Yes, we're making progress. But that's an independent thing, partially constrained by implementation complexity, partially constrained by cross platform worries. You're free to work on reducing the dangers around other variables, but I don't understand in the very least why that should stop this feature. > I'm not entirely sure how wal_level has the same danger as the > others.. Try setting max_wal_senders = 10 and wal_level = minimal. > > > There is absolutely no question in my mind that this will be a huge > > > support pain, from the first "ALTER SYSTEM SET shared_buffers = blah; > > > SHOW shared_buffers;" to the "why can't my database start?!? it's > > > complaining it can't allocate memory but I keep changing postgresql.conf > > > and nothing works!" I'm simply not convinced that this is moving us > > > forward nor that we will end up with more benefit than pain from it. > > > > That will not show the changed shared_buffers. And it (afair) will throw > > a WARNING that shared_buffers couldn't be adjusted at this point. > > Not showing the change is what I was getting at. As has been said > elsewhere, throwing a warning on every interesting invokation of a > command can speak to it not being exactly 'ready'. Then pg isn't ready already, because it has done that for a long time. Set some PGC_POSTMASTER variable and reload the configuration. Guiding a user that to perform required further action doesn't imply nonreadyness in the least. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Robert Haas (robertmh...@gmail.com) wrote: > there's a fairly generous but fixed-at-startup-time limit on how many > segments you can have. In practice I don't think this matters much, > but it was a sobering reminder that the main shared memory segment, > with all of its inflexibility, has important reliability properties > that are hard to replicate in more dynamic scenarios. Why wouldn't it be possible to have the same arrangment for shared_buffers, where you have more entires than you 'need' at startup but which then allows you to add more shared segments later? I do see that we would need an additional bit of indirection to handle that, which might be too much overhead, but the concept seems possible. Isn't that more-or-less how the kernel handles dynamic memory..? > Under the currently-proposed design, it can't be used to do any such > thing. It can only be used to modify some auto.conf file which lives > in $PGDATA. It's therefore no different from the ops perspective than > ALTER DATABASE or ALTER USER - except that it allows all settings to > be changed rather than only a subset. Claiming that modifying a file *included from a file in /etc* doesn't modify things under /etc is disingenuous, imv. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > On 2013-08-30 09:19:42 -0400, Stephen Frost wrote: > > wal_level and shared_buffers I can buy, but listen_addresses? The most > > typical change there is going from localhost -> '*', but you've got to > > be on the box to do that. Anything else and you're going to need to be > > adding interfaces to the box anyway and hacking around in > > /etc/network/interfaces or what-have-you. > > Even if it requires to be on the box locally, I only need libpq for > it. And it's not infrequent to allow additional, already configured > interfaces. And even if not, what's the point of prohibiting > listen_interfaces specifically? When the other very interesting > variables have the same dangers? I'd like to see those dangers removed from the other very interesting variables. We're making progress towards that, for example with shared_buffers. I've commented on that already up-thread. Hell, you could even make things such that PG would start w/ a misconfigured listen_addresses- but if we don't like that then I would argue that it shouldn't be included here. I'm not entirely sure how wal_level has the same danger as the others.. > Doing this on a variable-by-variable basis will a) be a ridiculous > amount of effort, b) confuse users which may not share our judgement of > individual variables. I don't think the effort involved is nearly as much as you claim and we already have the issue that users don't like our choices around what can be reconfigured on the fly and what can't (perhaps they understand that there are technical challenges to some of them, but that doesn't make them agree with them). > > You and Robert both seem to be of the opinion that this hack which > > brings postgresql.conf into the database via ALTER SYSTEM is a-ok > > because it's moving us "forward" in someone's mind, but it really is > > developing a system configuration management system which *looks* like a > > first-class citizen when it actually falls woefully short of that. > > It's what plenty of people want and it doesn't hurt people who do not > want it. Yes. I think that's a step forward. It will be quite interesting to see if people decide they really wanted this once they actually *get* it. > > There is absolutely no question in my mind that this will be a huge > > support pain, from the first "ALTER SYSTEM SET shared_buffers = blah; > > SHOW shared_buffers;" to the "why can't my database start?!? it's > > complaining it can't allocate memory but I keep changing postgresql.conf > > and nothing works!" I'm simply not convinced that this is moving us > > forward nor that we will end up with more benefit than pain from it. > > That will not show the changed shared_buffers. And it (afair) will throw > a WARNING that shared_buffers couldn't be adjusted at this point. Not showing the change is what I was getting at. As has been said elsewhere, throwing a warning on every interesting invokation of a command can speak to it not being exactly 'ready'. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Aug 30, 2013 at 8:53 AM, Stephen Frost wrote: > Yes, one of the issues with the existing system is that you can't > specify a default to be applied to new roles. Also, there are > parameters which are not per-role yet which it probably makes sense to > be in the database and we'd need a way to deal with that. Although, at > the same time, considering it role based does make for a nice > distinction. Unfortunately, those clammoring for this will argue that > it wouldn't replace adminpack and that they couldn't use it to modify > their /etc/network/interfaces file, which is the obvious next step to > all of this. This is a straw-man. adminpack doesn't allow reading or writing files outside of the configured data and log directories, as a security precaution. But suppose it did. If your permissions on /etc/network/interfaces allow the postgres user to modify it, then you pretty much deserve exactly what you get. Likening this to the feature being proposed is silly. What is being asked for here is the ability to easily modify system-wide settings from the SQL prompt, just as today you can modify settings per-user or per-database. That's not the same thing as rewriting the entire system configuration. -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 29, 2013 at 9:26 PM, Stephen Frost wrote: >> In the first place, modifying postgresql.conf and not immediately >> restarting the server to test your changes is probably the single most >> basic DBA error imaginable. > > You're not modifying postgresql.conf with ALTER SYSTEM, now are you? > Admins are going to realize the need to restart or at least reload the > service after updating such, but a DBA who has focused mainly on > normalization, query optimization, etc, isn't going to have the same > experience that a sysadmin has. I refuse to reject any feature on the basis of the possibility that people might use it without reading the documentation. Nearly anything anyone will ever propose is so idiot-proof that we can't conceive of a scenario in which someone shoots their foot off with it. The question is whether the usefulness of the feature exceeds, by a sufficient amount, the harm it's likely to do, and I think the answer is clearly yes. > A DBA updating a GUC, something they are likely to do frequently > day-in and day-out, isn't necessairly going to consider that it's a > reboot-needing change. Indeed, it's not unreasonable to consider that > we may, some day, actually be able to change or increase > shared_buffers on the fly (or error in the trying); I'd think your > work with the dynamic backends would actually make that likely to > happen sooner rather than later. I wouldn't hold my breath. We have way too many separate, variable-length data structures in shared memory. Increasing shared_buffers means making the lwlock array bigger, making the buffer header array bigger, allocating more space for the buffer mapping hash, etc. I doubt we'd accept a design that involves each of those data structures being a separate shared memory segment, but if they're all in the same segment one after another, then you can't easily extend them on the fly. There are also a lot of problems around unmapping-and-remapping a segment. If the remap fails, it's going to be at least a FATAL, but if you had any shared memory state in the unmapped segment (like a buffer pin) that has to be unwound, you have to PANIC; there's no other way to fix it. My initial design for dynamic shared memory allowed for an unbounded number of dynamic shared memory segments by growing the control segment on the fly. However, this introduced PANIC hazards in corner cases that I couldn't get rid of, so now there's a fairly generous but fixed-at-startup-time limit on how many segments you can have. In practice I don't think this matters much, but it was a sobering reminder that the main shared memory segment, with all of its inflexibility, has important reliability properties that are hard to replicate in more dynamic scenarios. > It's not the OPs guy that I'm worried about using ALTER SYSTEM- I don't > expect them to have any clue about it or care about it, except where it > can be used to modify things under /etc which they, rightfully, consider > their domain. Under the currently-proposed design, it can't be used to do any such thing. It can only be used to modify some auto.conf file which lives in $PGDATA. It's therefore no different from the ops perspective than ALTER DATABASE or ALTER USER - except that it allows all settings to be changed rather than only a subset. -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-30 09:19:42 -0400, Stephen Frost wrote: > * Andres Freund (and...@2ndquadrant.com) wrote: > > On 2013-08-30 08:48:21 -0400, Stephen Frost wrote: > > > I really just don't buy that- I've already put forward suggestions for > > > how to deal with it, but no one here seems to understand the > > > distinction. Modifying listen_addresses through ALTER SYSTEM is akin to > > > ISC/bind allowing changes to its listen_addresses equivilant through > > > dynamic DNS updates. Would it be possible to implement? Sure. Does it > > > make any sense? Certainly not. > > > > I very much want to change stuff like wal_level, listen_addresses and > > shared_buffers via ALTER SYSTEM. Configuration variables like that > > (PGC_POSTMASTER stuff mostly) are the prime reason why you actually need > > to change postgresql.conf instead of changing per user/database > > settings. > > wal_level and shared_buffers I can buy, but listen_addresses? The most > typical change there is going from localhost -> '*', but you've got to > be on the box to do that. Anything else and you're going to need to be > adding interfaces to the box anyway and hacking around in > /etc/network/interfaces or what-have-you. Even if it requires to be on the box locally, I only need libpq for it. And it's not infrequent to allow additional, already configured interfaces. And even if not, what's the point of prohibiting listen_interfaces specifically? When the other very interesting variables have the same dangers? Doing this on a variable-by-variable basis will a) be a ridiculous amount of effort, b) confuse users which may not share our judgement of individual variables. > > > Because we've got crap mixed into postgresql.conf which are bootstrap > > > configs needed to get the system started. Those things, in my view > > > anyway, fall much more into the category of "resources which should be > > > managed outside the database" than pg_hba.conf. > > > > I think the problem with your position in this thread is that you want > > to overhaul the way our configuration works in a pretty radical > > way. Which is fair enough, there certainly are deficiencies. But it's > > not the topic of this thread. > > You and Robert both seem to be of the opinion that this hack which > brings postgresql.conf into the database via ALTER SYSTEM is a-ok > because it's moving us "forward" in someone's mind, but it really is > developing a system configuration management system which *looks* like a > first-class citizen when it actually falls woefully short of that. It's what plenty of people want and it doesn't hurt people who do not want it. Yes. I think that's a step forward. > There is absolutely no question in my mind that this will be a huge > support pain, from the first "ALTER SYSTEM SET shared_buffers = blah; > SHOW shared_buffers;" to the "why can't my database start?!? it's > complaining it can't allocate memory but I keep changing postgresql.conf > and nothing works!" I'm simply not convinced that this is moving us > forward nor that we will end up with more benefit than pain from it. That will not show the changed shared_buffers. And it (afair) will throw a WARNING that shared_buffers couldn't be adjusted at this point. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > On 2013-08-29 21:26:48 -0400, Stephen Frost wrote: > > It's not the OPs guy that I'm worried about using ALTER SYSTEM- I don't > > expect them to have any clue about it or care about it, except where it > > can be used to modify things under /etc which they, rightfully, consider > > their domain. > > I think for the scenarios you describe it makes far, far much more sense > to add the ability to easily monitor for two things: > * on-disk configuration isn't the same as the currently loaded (not > trivially possible yet) That would certainly help. I don't know that it needs to be technically 'trivial', but at least having check_postgres.pl or something which can alert on it would be an improvement. Clearly, that would be valuable today (assuming it doesn't already exist somewhere.. it might). > * Configuration variables only come from locations that are approved for > in your scenario (Already possible, we might want to make it even easier) That an interesting notion; do you have something specific in mind..? The easiest, imv anyway, would be that options set in postgresql.conf can't be overridden, but that gets us into the bootstrap problem that people seem to be concerned about. It would also be a change to how postgresql.conf is parsed today which some people would be annoyed by. Having some configuration option which says what can be modified by alter system doesn't strike me as a terribly good solution either. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > On 2013-08-30 08:48:21 -0400, Stephen Frost wrote: > > I really just don't buy that- I've already put forward suggestions for > > how to deal with it, but no one here seems to understand the > > distinction. Modifying listen_addresses through ALTER SYSTEM is akin to > > ISC/bind allowing changes to its listen_addresses equivilant through > > dynamic DNS updates. Would it be possible to implement? Sure. Does it > > make any sense? Certainly not. > > I very much want to change stuff like wal_level, listen_addresses and > shared_buffers via ALTER SYSTEM. Configuration variables like that > (PGC_POSTMASTER stuff mostly) are the prime reason why you actually need > to change postgresql.conf instead of changing per user/database > settings. wal_level and shared_buffers I can buy, but listen_addresses? The most typical change there is going from localhost -> '*', but you've got to be on the box to do that. Anything else and you're going to need to be adding interfaces to the box anyway and hacking around in /etc/network/interfaces or what-have-you. > > Because we've got crap mixed into postgresql.conf which are bootstrap > > configs needed to get the system started. Those things, in my view > > anyway, fall much more into the category of "resources which should be > > managed outside the database" than pg_hba.conf. > > I think the problem with your position in this thread is that you want > to overhaul the way our configuration works in a pretty radical > way. Which is fair enough, there certainly are deficiencies. But it's > not the topic of this thread. You and Robert both seem to be of the opinion that this hack which brings postgresql.conf into the database via ALTER SYSTEM is a-ok because it's moving us "forward" in someone's mind, but it really is developing a system configuration management system which *looks* like a first-class citizen when it actually falls woefully short of that. There is absolutely no question in my mind that this will be a huge support pain, from the first "ALTER SYSTEM SET shared_buffers = blah; SHOW shared_buffers;" to the "why can't my database start?!? it's complaining it can't allocate memory but I keep changing postgresql.conf and nothing works!" I'm simply not convinced that this is moving us forward nor that we will end up with more benefit than pain from it. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 29, 2013 at 9:12 PM, Stephen Frost wrote: > You will not find me argueing to allow that in normal operation, or most > direct-catalog hacking. I'd be much happier if we had all the ALTER, > etc, options necessary to prevent any need to ever touch the catalogs. > Similairly, it'd be nice to have more permission options which reduce > the need for anyone to be superuser. Sure, I agree. But you can't burden this patch with the job of conforming to what you and I think the project policy ought to be, but isn't. Right now, as things stand today, the superuser is one step short of God Almighty. The only way in which we even attempt to restrict the superuser is to try to keep her from hijacking the postgres login account. But we don't even try to do that very thoroughly, as has recently been pointed out on other threads; there are several plausible attack vectors for her to do just that. This patch fits right into that existing philosophy. If we take this patch, and I think we should, and later change our policy, then the permissions around this may require adjustment to fit the new policy. Fine! But that policy change is properly a separate discussion. >> Now, you can argue that people are more likely to render the database >> nonfunctional by changing GUC settings than they are to do it by >> corrupting the system catalogs, but I'm not sure I believe it. We >> can, after all, validate that any setting a user supplies is a valid >> value for that setting before writing it out to the configuration >> file. It might still make the system fail to start if - for example - >> it causes too many semaphores to be reserved, or something like that. >> But why should we think that such mistakes will be common? If >> anything, it sounds less error-prone to me than hand-editing the >> configuration file, where typing something like on_exit_error=maybe >> will make the server fail to start. We can eliminate that whole >> category of error, at least for people who choose to use the new >> tools. > > That category of error is much more likely to get caught by the sysadmin > doing the restart after the update.. If you edit postgresql.conf manually today, you will have no warning of ANY sort of error you might make. With a feature like this, we can catch a large subset of things you might do wrong *before we even modify the file*. Yes, there will be some that slip through, but an imperfect mechanism for catching mistakes is better than no mechanism at all. > ALTER SYSTEM doesn't provide any > way to restart the DB to make sure it worked. > That, in turn, encourages > modification of the config parameters w/o a restart, which is a pretty > bad idea, imv. "vi" doesn't provide any way to restart the DB to make sure it worked, either, and never will. However, ALTER SYSTEM could be extended in exactly that way: we could have ALTER SYSTEM RESTART. Of course some people are likely to argue that's a bad idea, so we'd have to have that discussion and think carefully about the issues, but there's nothing intrinsic that restricts us from giving ALTER SYSTEM any arbitrary set of capabilities we might want it to have. Even if we don't ever do that, we're no worse off than today. Well, with one exception. If we make it easier to modify the configuration file, then people are more likely to do it, and thus more likely to do it wrong. But you could use that argument against any change that improves ease-of-use. And ease-of-use is a feature, not a bug. >> If you're using the term "dangerous" to refer to a security exposure, >> I concede there is some incremental exposure from allowing this by >> default. But it's not a terribly large additional exposure. It's >> already the case that if adminpack is available the super-user can do >> whatever he or she wants, because she or he can write to arbitrary >> files inside the data directory. > > This is only true if -contrib is installed, which a *lot* of admins > refuse to do, specifically because of such insane modules as this. > Therefore, I would argue that it's not nearly as small a change wrt > exposure as you believe. Strictly speaking, I don't believe it's a huge > security hole or something, but I do think it's going to surprise admins > who aren't following the lists. If we take the patch, it will be in the release notes, just like any other feature. I suspect there will be more people *pleasantly* surprised than anything else. If there's anything I've learned about ease-of-use as a 10+-year veteran of PostgreSQL, it's that being able to do everything via a database connection is really, really, really convenient. That's why foreign data wrappers are awesome. Instead of installing a foreign data wrapper for Oracle and pointing it at an Oracle server and then sucking data down over the link to store or process or whatever, you put all that logic in a client which knows how to talk to both PostgreSQL and Oracle. It turns out, though, that having
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-29 21:26:48 -0400, Stephen Frost wrote: > > Sure, you can construct a scenario where this matters. The ops guys > > have "sudo postgres pg_ctl" access but adminpack isn't installed and > > they have no other way to modify the configuration file. But that's > > just bizarre. And if that's really the environment you have, then you > > can install a loadable module that grabs ProcessUtility_hook and uses > > it to forbid ALTER SYSTEM on that machine. Hell, we can ship such a > > thing in contrib. Problem solved. But it's surely too obscure a > > combination of circumstances to justify disabling this by default. > > It's not the OPs guy that I'm worried about using ALTER SYSTEM- I don't > expect them to have any clue about it or care about it, except where it > can be used to modify things under /etc which they, rightfully, consider > their domain. I think for the scenarios you describe it makes far, far much more sense to add the ability to easily monitor for two things: * on-disk configuration isn't the same as the currently loaded (not trivially possible yet) * Configuration variables only come from locations that are approved for in your scenario (Already possible, we might want to make it even easier) Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-30 08:48:21 -0400, Stephen Frost wrote: > * Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote: > > Stephen Frost writes: > > > The OPs people are the ones that will be upset with this because the DBAs > > > will be modifying configs which OPs rightfully claim as theirs. > > > > If that's the problem you want to solve, there's no technical solution > > that will put you at ease. That's a people and trust problem. > > I really just don't buy that- I've already put forward suggestions for > how to deal with it, but no one here seems to understand the > distinction. Modifying listen_addresses through ALTER SYSTEM is akin to > ISC/bind allowing changes to its listen_addresses equivilant through > dynamic DNS updates. Would it be possible to implement? Sure. Does it > make any sense? Certainly not. I very much want to change stuff like wal_level, listen_addresses and shared_buffers via ALTER SYSTEM. Configuration variables like that (PGC_POSTMASTER stuff mostly) are the prime reason why you actually need to change postgresql.conf instead of changing per user/database settings. And you don't even need to do anything special to implement it. Because it's already there. > > We currently have no way that I know of to disable ALTER ROLE SET and > > ALTER DATABASE SET effects, why do we need to provide that feature for > > ALTER SYSTEM SET so much? > > Because we've got crap mixed into postgresql.conf which are bootstrap > configs needed to get the system started. Those things, in my view > anyway, fall much more into the category of "resources which should be > managed outside the database" than pg_hba.conf. I think the problem with your position in this thread is that you want to overhaul the way our configuration works in a pretty radical way. Which is fair enough, there certainly are deficiencies. But it's not the topic of this thread. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Andres Freund (and...@2ndquadrant.com) wrote: > On 2013-08-30 10:20:48 +0200, Cédric Villemain wrote: > > Grammar can be added later when the feature is stable. > > Could you explain the advantages of this? It will require users to get > used to different interfaces and we will end up maintaining both just > about forever. And I don't see the grammar being that contentious? The "different interfaces" issue already exists, to some extent, as adminpack does exist already and this is clearly a different interface from that. That said, as I mentioned just now elsewhere, I agree that the grammar itself (being rather simple anyway...) isn't the contentious part of this. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Cédric Villemain (ced...@2ndquadrant.com) wrote: > ALTER ROLE ALL may be good enougth to handle every GUC that we can also > remove > from postgresql.conf (I suppose all GUC needing only a reload, not a > restart). > It may needs some improvement to handle changing default for ALL and adding > new role. Yes, one of the issues with the existing system is that you can't specify a default to be applied to new roles. Also, there are parameters which are not per-role yet which it probably makes sense to be in the database and we'd need a way to deal with that. Although, at the same time, considering it role based does make for a nice distinction. Unfortunately, those clammoring for this will argue that it wouldn't replace adminpack and that they couldn't use it to modify their /etc/network/interfaces file, which is the obvious next step to all of this. > Grammar can be added later when the feature is stable. I tend to agree w/ Andres on this point- the grammar isn't really the contentious part. I think I see where you were going with this in that excluding the grammar makes it able to live as a module, but that's a different consideration. > I've commented one of the proposed patch adding some helpers to validate GUC > change, I claimed this part was good enough to be added without ALTER SYSTEM > (so a contrib can use it). Perhaps.. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Dimitri Fontaine (dimi...@2ndquadrant.fr) wrote: > Stephen Frost writes: > > The OPs people are the ones that will be upset with this because the DBAs > > will be modifying configs which OPs rightfully claim as theirs. > > If that's the problem you want to solve, there's no technical solution > that will put you at ease. That's a people and trust problem. I really just don't buy that- I've already put forward suggestions for how to deal with it, but no one here seems to understand the distinction. Modifying listen_addresses through ALTER SYSTEM is akin to ISC/bind allowing changes to its listen_addresses equivilant through dynamic DNS updates. Would it be possible to implement? Sure. Does it make any sense? Certainly not. > I don't think typical (or less typical) organisation should drive our > technical choices too much, and I'm pretty confident they didn't in the > past: pg_hba.conf is a file not because it's meant for this or that team > but because it makes sense technically to manage the settings to allow > using some resources *outside* of said resources. I'm all for moving pg_hba.conf into the database properly, actually. We already handle permissions and user access in the DB and that's one of the DBA's responsibilities. The same goes for pg_ident.conf. > We currently have no way that I know of to disable ALTER ROLE SET and > ALTER DATABASE SET effects, why do we need to provide that feature for > ALTER SYSTEM SET so much? Because we've got crap mixed into postgresql.conf which are bootstrap configs needed to get the system started. Those things, in my view anyway, fall much more into the category of "resources which should be managed outside the database" than pg_hba.conf. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Hi, On 2013-08-30 10:20:48 +0200, Cédric Villemain wrote: > > > The energy wasted in a good part of this massive 550+ messages thread is > > > truly saddening. We all (c|sh)ould have spent that time making PG more > > > awesome instead. > > > > Perhaps not understood by all, but keeping PG awesome involves more than > > adding every feature proposed- it also means saying no sometimes; to > > features, to new GUCs, even to micro-optimizations when they're overly > > complicated and offer only minimal or questionable improvements. > > Agreed, the current feature and proposal does not include pg_reload, and it > introduces a full machinery we absolutely don't need. The complexity in the last version of the patch I looked at wasn't in the added grammar or pg_reload() (well, it didn't have that). It was the logic to read (from memory)/write the config file and validate the GUCs. That's needed even if you put it into some module. And it requires support from guc.c/guc-file.l > Grammar can be added later when the feature is stable. Could you explain the advantages of this? It will require users to get used to different interfaces and we will end up maintaining both just about forever. And I don't see the grammar being that contentious? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost writes: > The OPs people are the ones that will be upset with this because the DBAs > will be modifying configs which OPs rightfully claim as theirs. If that's the problem you want to solve, there's no technical solution that will put you at ease. That's a people and trust problem. I don't think typical (or less typical) organisation should drive our technical choices too much, and I'm pretty confident they didn't in the past: pg_hba.conf is a file not because it's meant for this or that team but because it makes sense technically to manage the settings to allow using some resources *outside* of said resources. We currently have no way that I know of to disable ALTER ROLE SET and ALTER DATABASE SET effects, why do we need to provide that feature for ALTER SYSTEM SET so much? 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Le jeudi 29 août 2013 18:42:13 Stephen Frost a écrit : > On Thursday, August 29, 2013, Andres Freund wrote: > > If you don't want your installation to use it, tell you ops people not > > to do so. They are superusers, they need to have the ability to follow > > some rules you make up internally. > > The OPs people are the ones that will be upset with this because the DBAs > will be modifying configs which OPs rightfully claim as theirs. If they > have a way to prevent it then perhaps it's not terrible but they'd also > need to know to disable this new "feature". As for ALTER DATABASE- I would > be happier with encouraging use of that (or providing an ALTER CLUSTER) for > those thing it makes sense and works for and removing those from being in > postgresql.conf. I still feel things like listen_addresses shouldn't be > changed through this. ALTER ROLE ALL may be good enougth to handle every GUC that we can also remove from postgresql.conf (I suppose all GUC needing only a reload, not a restart). It may needs some improvement to handle changing default for ALL and adding new role. > > The energy wasted in a good part of this massive 550+ messages thread is > > truly saddening. We all (c|sh)ould have spent that time making PG more > > awesome instead. > > Perhaps not understood by all, but keeping PG awesome involves more than > adding every feature proposed- it also means saying no sometimes; to > features, to new GUCs, even to micro-optimizations when they're overly > complicated and offer only minimal or questionable improvements. Agreed, the current feature and proposal does not include pg_reload, and it introduces a full machinery we absolutely don't need. Grammar can be added later when the feature is stable. So far, we can achieve the goal by using adminpack, by using a file_fdw or a config_fdw. IMHO it is the job of a FDW to be able to handle atomic write or anything like that. I've commented one of the proposed patch adding some helpers to validate GUC change, I claimed this part was good enough to be added without ALTER SYSTEM (so a contrib can use it). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 29, 2013 at 9:13 AM, Robert Haas wrote: > I think the goals of this patch should be to (1) let users of other > clients manipulate the startup configuration just as easily as we can > already do using pgAdmin and (2) remove some of the concurrency > hazards that pgAdmin introduces, for example by using locking and > atomic renames. +1 -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Robert Haas (robertmh...@gmail.com) wrote: > On Thu, Aug 29, 2013 at 8:07 PM, Stephen Frost wrote: > > I'm not talking about malicious DBAs but rather a generally > > knowledgable DBA who changed shared_buffers up too high and then leaves on > > vacation, while the OPs guys need to do a database restart for whatever > > reason and then discover it doesn't start. > > /me looks at Stephen incredulously. > > In the first place, modifying postgresql.conf and not immediately > restarting the server to test your changes is probably the single most > basic DBA error imaginable. You're not modifying postgresql.conf with ALTER SYSTEM, now are you? Admins are going to realize the need to restart or at least reload the service after updating such, but a DBA who has focused mainly on normalization, query optimization, etc, isn't going to have the same experience that a sysadmin has. A DBA updating a GUC, something they are likely to do frequently day-in and day-out, isn't necessairly going to consider that it's a reboot-needing change. Indeed, it's not unreasonable to consider that we may, some day, actually be able to change or increase shared_buffers on the fly (or error in the trying); I'd think your work with the dynamic backends would actually make that likely to happen sooner rather than later. > I have a hard time viewing such a person > as generally knowledgeable. I hope the DBA in question doesn't have > access to the switches, because he's probably the sort of guy who > reassigns switch ports and doesn't type "wr m" afterwards. I'd consider the DBAs who I work with as generally knowledgable, and thankfully also cautious enough to talk to me before making changes, but they have superuser access (they need to be able to do database 'releases' to our production environments and those scripts need to change table ownership and do other things which can be annoying to maintain the permissions for) and certainly understand changing GUCs (mostly things like work_mem). > In the second place, the same guy can do the same thing today. He > just has to use "vi". In fact, I think this is a pretty common > failure mode in poorly-controlled environments where too many people > have access to the configuration files. Now maybe you're going to > tell me that the ops guys can't modify the configuration file because > they only have SQL-level access, but then how are they managing to > restart the database? They need to be able to run pg_ctl *as the > postgres user* to do that, and if they have shell access to that > account, all bets are off. You seem to be confused here between the DBA/data team and the OPs or Infrastructure team. I'm making a clear distinction between them and feel quite comfortable with it because it's the environment that I live in today. My job, in fact, is exactly to straddle that line and work with both. In the above example, the DBA hasn't got root on the box and can't simply go change or modify postgresql.conf, not even with "vi", and is true for most of the DBAs on my team, even though they have superuser access in PG. > Sure, you can construct a scenario where this matters. The ops guys > have "sudo postgres pg_ctl" access but adminpack isn't installed and > they have no other way to modify the configuration file. But that's > just bizarre. And if that's really the environment you have, then you > can install a loadable module that grabs ProcessUtility_hook and uses > it to forbid ALTER SYSTEM on that machine. Hell, we can ship such a > thing in contrib. Problem solved. But it's surely too obscure a > combination of circumstances to justify disabling this by default. It's not the OPs guy that I'm worried about using ALTER SYSTEM- I don't expect them to have any clue about it or care about it, except where it can be used to modify things under /etc which they, rightfully, consider their domain. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Robert, Was working on replying to this, but got distracted.. * Robert Haas (robertmh...@gmail.com) wrote: > To be honest, I think the argument that this is dangerous is pretty > ridiculous. AFAICS, the only argument anyone's advanced for this > being dangerous is the theory that you might accidentally put > something in your postgresql.conf file that makes the server not > start. However, the reality is that the superuser has MANY, MANY ways > of killing the database cluster as things stand. Consider the > ever-popular "DELETE FROM pg_proc". You will not find me argueing to allow that in normal operation, or most direct-catalog hacking. I'd be much happier if we had all the ALTER, etc, options necessary to prevent any need to ever touch the catalogs. Similairly, it'd be nice to have more permission options which reduce the need for anyone to be superuser. > Now, you can argue that people are more likely to render the database > nonfunctional by changing GUC settings than they are to do it by > corrupting the system catalogs, but I'm not sure I believe it. We > can, after all, validate that any setting a user supplies is a valid > value for that setting before writing it out to the configuration > file. It might still make the system fail to start if - for example - > it causes too many semaphores to be reserved, or something like that. > But why should we think that such mistakes will be common? If > anything, it sounds less error-prone to me than hand-editing the > configuration file, where typing something like on_exit_error=maybe > will make the server fail to start. We can eliminate that whole > category of error, at least for people who choose to use the new > tools. That category of error is much more likely to get caught by the sysadmin doing the restart after the update.. ALTER SYSTEM doesn't provide any way to restart the DB to make sure it worked. That, in turn, encourages modification of the config parameters w/o a restart, which is a pretty bad idea, imv. > If you're using the term "dangerous" to refer to a security exposure, > I concede there is some incremental exposure from allowing this by > default. But it's not a terribly large additional exposure. It's > already the case that if adminpack is available the super-user can do > whatever he or she wants, because she or he can write to arbitrary > files inside the data directory. This is only true if -contrib is installed, which a *lot* of admins refuse to do, specifically because of such insane modules as this. Therefore, I would argue that it's not nearly as small a change wrt exposure as you believe. Strictly speaking, I don't believe it's a huge security hole or something, but I do think it's going to surprise admins who aren't following the lists. > Even if not, for most intents and > purposes, ALTER DATABASE my_main_database SET whatever = thing is > functionally equivalent to modifying postgresql.conf. Some settings > can't be modified that way, but so what? AFAICS, about the worst > thing the user can do is ALTER SYSTEM SET shared_preload_libraries = > 'rootkit'. This is assuming a malicious user, and one who has access to the filesystem, which are pretty big assumptions to be making. I'm not trying to make any argument about a malicious user and in general I don't believe the users of this capability are people who have root on the box. > Huh? The problem with adminpack is that it doesn't let you modify > individual configuration settings. All you can do is rewrite an > entire file. I guess somebody could write a specialized client that > just uses that infrastructure to rewrite postgresql.conf. For all I > know, someone has. Even if not, I don't think that you can use that > to prove that people don't care about this feature. If nobody cares, > why are there 400 emails on this topic?! Addressed this in my reply to Andres. > > Why can't adminpack do that..? > > It could. But it doesn't. We could improve it to do that, and that > might be worthwhile, but it still wouldn't be as nice as what's being > proposed here. This wouldn't be the first time we've said "do it externally and when we figure out a better answer then we'll put it into core." > 1. disabling the feature by default, and providing no way for it to be > turned on remotely, and Yes, that's quite intentional because it would make someone knowledgable about the *overall system* be aware of this rather than having it sprung on them when they discover some change has made the database unable to start. > 2. even when the feature is enabled, only allowing a subset of the > settings to be changed with it, and > 3. also changing the interpretation of the config files so that we > have a first-wins rules instead of a last-wins rule. > > If we do either #1 or #2, this won't be a plausible substitute for the > functionality that adminpack offers today. While I appreciate that the original thrust behind this may be trying to replace
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 29, 2013 at 8:07 PM, Stephen Frost wrote: > Having 400 emails about it means it's contentious. That's quite different > from having a large demand. It does speak to the author's persistence as > well, but that shouldn't be a surprise. Yet you can't ignore the fact that many people on these threads want some form of this. >> Presumably one major reason why we don't have other|good GUIs is that >> it's ridicuously hard to make them work to an interesting extent with >> the current infrastructure. > > Yet no one has tried to improve admin pack? No, but multiple people have tried to do ALTER SYSTEM .. SET. Peter had a crack at this problem, I fooled around with it (though I don't think I ever got as far as publishing), and there were various others as well (Greg Smith?). > I'm not talking about malicious DBAs but rather a generally > knowledgable DBA who changed shared_buffers up too high and then leaves on > vacation, while the OPs guys need to do a database restart for whatever > reason and then discover it doesn't start. /me looks at Stephen incredulously. In the first place, modifying postgresql.conf and not immediately restarting the server to test your changes is probably the single most basic DBA error imaginable. I have a hard time viewing such a person as generally knowledgeable. I hope the DBA in question doesn't have access to the switches, because he's probably the sort of guy who reassigns switch ports and doesn't type "wr m" afterwards. In the second place, the same guy can do the same thing today. He just has to use "vi". In fact, I think this is a pretty common failure mode in poorly-controlled environments where too many people have access to the configuration files. Now maybe you're going to tell me that the ops guys can't modify the configuration file because they only have SQL-level access, but then how are they managing to restart the database? They need to be able to run pg_ctl *as the postgres user* to do that, and if they have shell access to that account, all bets are off. Sure, you can construct a scenario where this matters. The ops guys have "sudo postgres pg_ctl" access but adminpack isn't installed and they have no other way to modify the configuration file. But that's just bizarre. And if that's really the environment you have, then you can install a loadable module that grabs ProcessUtility_hook and uses it to forbid ALTER SYSTEM on that machine. Hell, we can ship such a thing in contrib. Problem solved. But it's surely too obscure a combination of circumstances to justify disabling this by default. -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thursday, August 29, 2013, Andres Freund wrote: > > To quote Robert two mails up: > > > Huh? The problem with adminpack is that it doesn't let you modify > > individual configuration settings. All you can do is rewrite an > > entire file. That's clearly fixable. > I guess somebody could write a specialized client that > > just uses that infrastructure to rewrite postgresql.conf. For all I > > know, someone has. Even if not, I don't think that you can use that > > to prove that people don't care about this feature. If nobody cares, > > why are there 400 emails on this topic?! Having 400 emails about it means it's contentious. That's quite different from having a large demand. It does speak to the author's persistence as well, but that shouldn't be a surprise. > Also, doing it the adminpack way lacks even the most basic validity > checks. And that's not really changeable. I don't see why..? Admin pack could certainly be modified to take a parameter and do appropriate verification before locking an object and rewriting the file. It's what we're being expected to do in core, after all. Indeed, we can't even do validity checks on all the options, which is the crux of what I'm concerned about. > Presumably one major reason why we don't have other|good GUIs is that > it's ridicuously hard to make them work to an interesting extent with > the current infrastructure. Yet no one has tried to improve admin pack? > If they give out superuser access it has to be to people who can follow > rules. After all they don't DROP DATABASE; DELETE FROM pg_class; alter > passwords; use adminpack (changing postgresql.conf..); ... All of which > they can do. > This completely misses, or perhaps just ignores, the point. Disallowing super user access can be difficult because there's a lot of *normal* DBA activities which can't be easily done without it (like changing table ownership or similar). The "createrole" option definitely improved things but we aren't there yet. It's certainly easy to simply not install the adminpack. The other concerns above are strawmen because they attack a malicious DBA. I'm not talking about malicious DBAs but rather a generally knowledgable DBA who changed shared_buffers up too high and then leaves on vacation, while the OPs guys need to do a database restart for whatever reason and then discover it doesn't start. I bring up these concerns because I have environments where I can see exactly this happening and I have a hard time believing that I'm somehow alone. Thanks, Stephen
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-29 18:42:13 -0400, Stephen Frost wrote: > On Thursday, August 29, 2013, Andres Freund wrote: > > The energy wasted in a good part of this massive 550+ messages thread is > > truly saddening. We all (c|sh)ould have spent that time making PG more > > awesome instead. > Perhaps not understood by all, but keeping PG awesome involves more than > adding every feature proposed- it also means saying no sometimes; to > features, to new GUCs, even to micro-optimizations when they're overly > complicated and offer only minimal or questionable improvements. But that doesn't mean that endlessly discussing in circles is a worthwile thing to do. The discussion essentially hasn't progressed towards concensus in the last months at all besides involving most active hackers at some point. If anything it has become more contentious. Obviously lots of people have strong opinions. Now we need to make decisions. Even if it ends up being ones I judge to be ridiculously bad. > > On 2013-08-29 15:07:35 -0400, Robert Haas wrote: > > > I don't really see a compelling reason why it can't or shouldn't be in > > > core. But having something better in contrib would still be an > > > improvement on the status quo. > > > > I don't see much argument for putting it into contrib. One class of > > users this will benefit is relatively new ones, possibly using some > > GUI. Adding some additional complexity for them to enable the feature > > seems pointless to me. > I keep wondering what this fantastic new GUI that isn't pgAdmin is, and why > it wouldn't be able to use the exact same mechanism that pgAdmin uses and > provides in a few simple steps to enable this. To quote Robert two mails up: > Huh? The problem with adminpack is that it doesn't let you modify > individual configuration settings. All you can do is rewrite an > entire file. I guess somebody could write a specialized client that > just uses that infrastructure to rewrite postgresql.conf. For all I > know, someone has. Even if not, I don't think that you can use that > to prove that people don't care about this feature. If nobody cares, > why are there 400 emails on this topic?! Also, doing it the adminpack way lacks even the most basic validity checks. And that's not really changeable. Presumably one major reason why we don't have other|good GUIs is that it's ridicuously hard to make them work to an interesting extent with the current infrastructure. > > If you don't want your installation to use it, tell you ops people not > > to do so. They are superusers, they need to have the ability to follow > > some rules you make up internally. > The OPs people are the ones that will be upset with this because the DBAs > will be modifying configs which OPs rightfully claim as theirs. If they > have a way to prevent it then perhaps it's not terrible but they'd also > need to know to disable this new "feature". As for ALTER DATABASE- I would > be happier with encouraging use of that (or providing an ALTER CLUSTER) for > those thing it makes sense and works for and removing those from being in > postgresql.conf. I still feel things like listen_addresses shouldn't be > changed through this. If they give out superuser access it has to be to people who can follow rules. After all they don't DROP DATABASE; DELETE FROM pg_class; alter passwords; use adminpack (changing postgresql.conf..); ... All of which they can do. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thursday, August 29, 2013, Andres Freund wrote: > On 2013-08-29 15:07:35 -0400, Robert Haas wrote: > > I don't really see a compelling reason why it can't or shouldn't be in > > core. But having something better in contrib would still be an > > improvement on the status quo. > > I don't see much argument for putting it into contrib. One class of > users this will benefit is relatively new ones, possibly using some > GUI. Adding some additional complexity for them to enable the feature > seems pointless to me. I keep wondering what this fantastic new GUI that isn't pgAdmin is, and why it wouldn't be able to use the exact same mechanism that pgAdmin uses and provides in a few simple steps to enable this. > If you don't want your installation to use it, tell you ops people not > to do so. They are superusers, they need to have the ability to follow > some rules you make up internally. The OPs people are the ones that will be upset with this because the DBAs will be modifying configs which OPs rightfully claim as theirs. If they have a way to prevent it then perhaps it's not terrible but they'd also need to know to disable this new "feature". As for ALTER DATABASE- I would be happier with encouraging use of that (or providing an ALTER CLUSTER) for those thing it makes sense and works for and removing those from being in postgresql.conf. I still feel things like listen_addresses shouldn't be changed through this. > The energy wasted in a good part of this massive 550+ messages thread is > truly saddening. We all (c|sh)ould have spent that time making PG more > awesome instead. > Perhaps not understood by all, but keeping PG awesome involves more than adding every feature proposed- it also means saying no sometimes; to features, to new GUCs, even to micro-optimizations when they're overly complicated and offer only minimal or questionable improvements. Thanks, Stephen
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On 2013-08-29 15:07:35 -0400, Robert Haas wrote: > On Thu, Aug 29, 2013 at 1:42 PM, Stephen Frost wrote: > > To be honest, I don't find the arguments of "pgAdmin does it badly" > > nor "psql users want this ability" (which I find difficult to believe) > > to be particularlly compelling reasons to have a dangerous > > implementation (even if it's better than 'adminpack' today) in core. > > I don't understand how you can find that difficult to believe. I'm a > psql user and I want it. Josh Berkus is a psql user and he wants it. > And there are numerous statements of support on these threads from > other people as well. The sheer volume of discussion on this topic, > and the fact that it has not gone away after years of wrangling, is a > clear indication that people do, in fact, want it. > > To be honest, I think the argument that this is dangerous is pretty > ridiculous. +waytoomuch. > > If it's in core rather than in contrib it's going to be deployed a great > > deal farther with a large increase in userbase. I've already stated > > that if this is in contrib that my concerns are much less. > > I don't really see a compelling reason why it can't or shouldn't be in > core. But having something better in contrib would still be an > improvement on the status quo. I don't see much argument for putting it into contrib. One class of users this will benefit is relatively new ones, possibly using some GUI. Adding some additional complexity for them to enable the feature seems pointless to me. If you don't want your installation to use it, tell you ops people not to do so. They are superusers, they need to have the ability to follow some rules you make up internally. The energy wasted in a good part of this massive 550+ messages thread is truly saddening. We all (c|sh)ould have spent that time making PG more awesome instead. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 29, 2013 at 1:42 PM, Stephen Frost wrote: > To be honest, I don't find the arguments of "pgAdmin does it badly" > nor "psql users want this ability" (which I find difficult to believe) > to be particularlly compelling reasons to have a dangerous > implementation (even if it's better than 'adminpack' today) in core. I don't understand how you can find that difficult to believe. I'm a psql user and I want it. Josh Berkus is a psql user and he wants it. And there are numerous statements of support on these threads from other people as well. The sheer volume of discussion on this topic, and the fact that it has not gone away after years of wrangling, is a clear indication that people do, in fact, want it. To be honest, I think the argument that this is dangerous is pretty ridiculous. AFAICS, the only argument anyone's advanced for this being dangerous is the theory that you might accidentally put something in your postgresql.conf file that makes the server not start. However, the reality is that the superuser has MANY, MANY ways of killing the database cluster as things stand. Consider the ever-popular "DELETE FROM pg_proc". That will not only render your database unusable, but it's a hell of a lot harder to recover from than anything you might do to postgresql.conf. Therefore, from where I'm sitting, this is like telling a head of state with the ability to launch nuclear weapons which will destroy the planet that he isn't allowed to bring his swiss army knife on board an aircraft. Now, you can argue that people are more likely to render the database nonfunctional by changing GUC settings than they are to do it by corrupting the system catalogs, but I'm not sure I believe it. We can, after all, validate that any setting a user supplies is a valid value for that setting before writing it out to the configuration file. It might still make the system fail to start if - for example - it causes too many semaphores to be reserved, or something like that. But why should we think that such mistakes will be common? If anything, it sounds less error-prone to me than hand-editing the configuration file, where typing something like on_exit_error=maybe will make the server fail to start. We can eliminate that whole category of error, at least for people who choose to use the new tools. If you're using the term "dangerous" to refer to a security exposure, I concede there is some incremental exposure from allowing this by default. But it's not a terribly large additional exposure. It's already the case that if adminpack is available the super-user can do whatever he or she wants, because she or he can write to arbitrary files inside the data directory. Even if not, for most intents and purposes, ALTER DATABASE my_main_database SET whatever = thing is functionally equivalent to modifying postgresql.conf. Some settings can't be modified that way, but so what? AFAICS, about the worst thing the user can do is ALTER SYSTEM SET shared_preload_libraries = 'rootkit'. But if the user has the ability to install rootkit.so, the sysadmin is already screwed. And aside from that sort of thing, I don't really see what can happen that's any worse than what a rogue superuser can already do as things stand today. > If it's in core rather than in contrib it's going to be deployed a great > deal farther with a large increase in userbase. I've already stated > that if this is in contrib that my concerns are much less. I don't really see a compelling reason why it can't or shouldn't be in core. But having something better in contrib would still be an improvement on the status quo. >> I think the goals of this patch should be to (1) let users of other >> clients manipulate the startup configuration just as easily as we can >> already do using pgAdmin, > > Which could be done already through use of adminpack.. The capabilities > exposed there could be used by other clients. The fact that none of the > other clients have chosen to do so could be an indication that this > ability isn't terribly 'in demand'. Huh? The problem with adminpack is that it doesn't let you modify individual configuration settings. All you can do is rewrite an entire file. I guess somebody could write a specialized client that just uses that infrastructure to rewrite postgresql.conf. For all I know, someone has. Even if not, I don't think that you can use that to prove that people don't care about this feature. If nobody cares, why are there 400 emails on this topic?! >> and (2) remove some of the concurrency >> hazards that pgAdmin introduces, for example by using locking and >> atomic renames. > > Why can't adminpack do that..? It could. But it doesn't. We could improve it to do that, and that might be worthwhile, but it still wouldn't be as nice as what's being proposed here. >> Restricting the functionality to something less than >> what pgAdmin provides will just cause people to ignore the new >> mechanism and use th
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Robert, * Robert Haas (robertmh...@gmail.com) wrote: > The sorts of watered-down half-features being proposed here are not > going to do anything to address that situation. If there are > restrictions on what GUCs can be changed with this feature, or if the > feature is disabled by default, or if you can only use it when the > moon is full and you hop on your left foot while spinning around > sideways, then pgAdmin (and any other, similar tools) are just going > to keep doing what they do today in the same crappy, unsafe way they > currently do it. Meanwhile, people who use psql are going to continue > to find themselves without a reasonable API for doing the same thing > that can be done easily using pgAdmin. To be honest, I don't find the arguments of "pgAdmin does it badly" nor "psql users want this ability" (which I find difficult to believe) to be particularlly compelling reasons to have a dangerous implementation (even if it's better than 'adminpack' today) in core. If it's in core rather than in contrib it's going to be deployed a great deal farther with a large increase in userbase. I've already stated that if this is in contrib that my concerns are much less. > I think the goals of this patch should be to (1) let users of other > clients manipulate the startup configuration just as easily as we can > already do using pgAdmin, Which could be done already through use of adminpack.. The capabilities exposed there could be used by other clients. The fact that none of the other clients have chosen to do so could be an indication that this ability isn't terribly 'in demand'. > and (2) remove some of the concurrency > hazards that pgAdmin introduces, for example by using locking and > atomic renames. Why can't adminpack do that..? > Restricting the functionality to something less than > what pgAdmin provides will just cause people to ignore the new > mechanism and use the one that already exists and, by and large, > works. And trying to revise other aspects of how GUCs and config > files work as part of this effort is not reasonably related to this > patch, and should be kept out of the discussion. We're talking about modifying config files through an interface and you wish to exclude all discussion about how those config files are handled. That leads to a result that only an adminpack-like solution is an option, to which I respond "use and improve adminpack then". If we want something that works well in *core* then I don't think we can exclude how core reads and handles config files from the discussion. We need a real solution, not another adminpack-like hack. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Aug 28, 2013 at 3:10 PM, Stephen Frost wrote: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Stephen Frost writes: >> > While I appreciate that there are bootstrap-type issues with this, I >> > really don't like this idea of "later stuff can just override earlier >> > stuff". >> >> > include files and conf.d-style options are for breaking the config up, >> > not to allow you to override options because a file came later than an >> > earlier file. Our particular implementation of config-file reading >> > happens to lend itself to later-definition-wins, but that's really >> > counter-intuitive for anyone unfamiliar with PG, imv. >> >> I don't follow this argument at all. Do you know any software with text >> config files that will act differently from this if the same setting is >> listed twice? "Last one wins" is certainly what I'd expect. > > Have you tried having the same mount specified in multiple files in > fstab.d..? Also, aiui (not a big exim fan personally), duplicate > definitions in an exim4/conf.d would result in an error. Playing around > in apache2/sites-enabled, it appears to be "first wins" wrt virtual > hosts. > > There's a number of cases where only a single value is being set and > subseqeunt files are 'additive' (eg: ld.so.conf.d), so they don't have > this issue. Similar to that are script directories, which simply run a > set of scripts in the $DIR.d and, as it's the scripts themselves which > are the 'parameters', and being files in a directory, you can't > duplicate them. Others (eg: pam.d) define the file name to be an > enclosing context, again preventing duplication by using the filename > itself. > > There are counter-examples also; sysctl.d will replace what's already > been set, so perhaps it simply depends on an individual's experience. > For my part, I'd much prefer an error or warning saying "you've got > duplicate definitions of X" than a last-wins approach, though perhaps > that's just because I like things to be very explicit and clear. > Allowing multiple definitions of the same parameter to be valid isn't > 'clear' to me. This may be true, but I think it's got little if anything to do with the current patch. There are lots of things that I don't like about our GUC machinery, too, but the goal of this thread shouldn't be to fix them all, but to get a useful piece of functionality added. The thing that we should keep in mind here is that it's *already* possible - TODAY - for users to overwrite postgresql.conf with a new set of settings. pgAdmin has this functionality built-in; it only requires that you install the "adminpack" contrib module, which we ship. According to Dave Page, pgAdmin will even offer to run CREATE EXTENSION adminpack for you, if you haven't done that already. Also according to Dave Page, if two users try to use it concurrently, you will get exactly the sort of awful results that you might expect. If it gets killed halfway through rewriting the file, too bad for you. The sorts of watered-down half-features being proposed here are not going to do anything to address that situation. If there are restrictions on what GUCs can be changed with this feature, or if the feature is disabled by default, or if you can only use it when the moon is full and you hop on your left foot while spinning around sideways, then pgAdmin (and any other, similar tools) are just going to keep doing what they do today in the same crappy, unsafe way they currently do it. Meanwhile, people who use psql are going to continue to find themselves without a reasonable API for doing the same thing that can be done easily using pgAdmin. I think the goals of this patch should be to (1) let users of other clients manipulate the startup configuration just as easily as we can already do using pgAdmin and (2) remove some of the concurrency hazards that pgAdmin introduces, for example by using locking and atomic renames. Restricting the functionality to something less than what pgAdmin provides will just cause people to ignore the new mechanism and use the one that already exists and, by and large, works. And trying to revise other aspects of how GUCs and config files work as part of this effort is not reasonably related to this patch, and should be kept out of the discussion. -- 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > There are counter-examples also; sysctl.d will replace what's already > been set, so perhaps it simply depends on an individual's experience. > For my part, I'd much prefer an error or warning saying "you've got > duplicate definitions of X" than a last-wins approach, though perhaps > that's just because I like things to be very explicit and clear. > Allowing multiple definitions of the same parameter to be valid isn't > 'clear' to me. One of the elements that had to be in place for this whole thing of multiple configuration files to be allowed was for pg_settings to include precise information about, for each setting, where is its value coming from. I doubt any of the systems you mentioned have comparable functionality. If the admin has trouble figuring it out, he needs only look into that view. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Aug 28, 2013 at 03:15:14PM -0400, Stephen Frost wrote: > * Bruce Momjian (br...@momjian.us) wrote: > > Agreed, but I think this is a much larger issue than ALTER SYSTEM SET. > > Yeah, true. > > > I think changing behavior to first-seen would only add to confusion. > > What we really need is a WARNING when a later postgresql.conf setting > > overrides an earlier one, and ALTER SYSTEM SET's config file could > > behave the same, so you would know right away when you were overriding > > something in postgresql.conf that appeared earlier. > > A warning would be nice. If only everyone read the log files. :/ I would expect ALTER SYSTEM SET to return a WARNING in such cases too. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Bruce Momjian (br...@momjian.us) wrote: > Agreed, but I think this is a much larger issue than ALTER SYSTEM SET. Yeah, true. > I think changing behavior to first-seen would only add to confusion. > What we really need is a WARNING when a later postgresql.conf setting > overrides an earlier one, and ALTER SYSTEM SET's config file could > behave the same, so you would know right away when you were overriding > something in postgresql.conf that appeared earlier. A warning would be nice. If only everyone read the log files. :/ Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > While I appreciate that there are bootstrap-type issues with this, I > > really don't like this idea of "later stuff can just override earlier > > stuff". > > > include files and conf.d-style options are for breaking the config up, > > not to allow you to override options because a file came later than an > > earlier file. Our particular implementation of config-file reading > > happens to lend itself to later-definition-wins, but that's really > > counter-intuitive for anyone unfamiliar with PG, imv. > > I don't follow this argument at all. Do you know any software with text > config files that will act differently from this if the same setting is > listed twice? "Last one wins" is certainly what I'd expect. Have you tried having the same mount specified in multiple files in fstab.d..? Also, aiui (not a big exim fan personally), duplicate definitions in an exim4/conf.d would result in an error. Playing around in apache2/sites-enabled, it appears to be "first wins" wrt virtual hosts. There's a number of cases where only a single value is being set and subseqeunt files are 'additive' (eg: ld.so.conf.d), so they don't have this issue. Similar to that are script directories, which simply run a set of scripts in the $DIR.d and, as it's the scripts themselves which are the 'parameters', and being files in a directory, you can't duplicate them. Others (eg: pam.d) define the file name to be an enclosing context, again preventing duplication by using the filename itself. There are counter-examples also; sysctl.d will replace what's already been set, so perhaps it simply depends on an individual's experience. For my part, I'd much prefer an error or warning saying "you've got duplicate definitions of X" than a last-wins approach, though perhaps that's just because I like things to be very explicit and clear. Allowing multiple definitions of the same parameter to be valid isn't 'clear' to me. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Aug 28, 2013 at 02:30:41PM -0400, Stephen Frost wrote: > * Bruce Momjian (br...@momjian.us) wrote: > > On Tue, Aug 27, 2013 at 09:04:00AM +0530, Amit Kapila wrote: > > > > I really hate the idea that someone could configure 'X' in > > > > postgresql.conf and because the auto.conf line is later in the file, > > > > it's able to override the original setting. Does not strike me as > > > > intuitive at all. > > > > > > This is currently how include mechanism works in postgresql.conf, > > > changing that for this special case can be costly and moreover the > > > specs for this patch were layout from beginning that way. > > > > Agreed. If you are worried about ALTER SYSTEM changing postgresql.conf > > settings, you should move the include_auto line to the top of > > postgresql.conf, but I don't think that should be the default. > > While I appreciate that there are bootstrap-type issues with this, I > really don't like this idea of "later stuff can just override earlier > stuff". > > include files and conf.d-style options are for breaking the config up, > not to allow you to override options because a file came later than an > earlier file. Our particular implementation of config-file reading > happens to lend itself to later-definition-wins, but that's really > counter-intuitive for anyone unfamiliar with PG, imv. Agreed, but I think this is a much larger issue than ALTER SYSTEM SET. I think changing behavior to first-seen would only add to confusion. What we really need is a WARNING when a later postgresql.conf setting overrides an earlier one, and ALTER SYSTEM SET's config file could behave the same, so you would know right away when you were overriding something in postgresql.conf that appeared earlier. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost writes: > While I appreciate that there are bootstrap-type issues with this, I > really don't like this idea of "later stuff can just override earlier > stuff". > include files and conf.d-style options are for breaking the config up, > not to allow you to override options because a file came later than an > earlier file. Our particular implementation of config-file reading > happens to lend itself to later-definition-wins, but that's really > counter-intuitive for anyone unfamiliar with PG, imv. I don't follow this argument at all. Do you know any software with text config files that will act differently from this if the same setting is listed twice? "Last one wins" is certainly what I'd expect. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Bruce Momjian (br...@momjian.us) wrote: > On Tue, Aug 27, 2013 at 09:04:00AM +0530, Amit Kapila wrote: > > > I really hate the idea that someone could configure 'X' in > > > postgresql.conf and because the auto.conf line is later in the file, > > > it's able to override the original setting. Does not strike me as > > > intuitive at all. > > > > This is currently how include mechanism works in postgresql.conf, > > changing that for this special case can be costly and moreover the > > specs for this patch were layout from beginning that way. > > Agreed. If you are worried about ALTER SYSTEM changing postgresql.conf > settings, you should move the include_auto line to the top of > postgresql.conf, but I don't think that should be the default. While I appreciate that there are bootstrap-type issues with this, I really don't like this idea of "later stuff can just override earlier stuff". include files and conf.d-style options are for breaking the config up, not to allow you to override options because a file came later than an earlier file. Our particular implementation of config-file reading happens to lend itself to later-definition-wins, but that's really counter-intuitive for anyone unfamiliar with PG, imv. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 27, 2013 at 09:04:00AM +0530, Amit Kapila wrote: > > For my part, I'd honestly rather have the first things found be what's > > picked and later things be ignored. If you want it controlled by ALTER > > SYSTEM, then don't set it in postgresql.conf. The problem with that is > > there's no "bootstrap" mechanism without actually modifying such configs > > or making the configs be in auto.conf to begin with, neither of which is > > ideal, imv. > > > > I really hate the idea that someone could configure 'X' in > > postgresql.conf and because the auto.conf line is later in the file, > > it's able to override the original setting. Does not strike me as > > intuitive at all. > > This is currently how include mechanism works in postgresql.conf, > changing that for this special case can be costly and moreover the > specs for this patch were layout from beginning that way. Agreed. If you are worried about ALTER SYSTEM changing postgresql.conf settings, you should move the include_auto line to the top of postgresql.conf, but I don't think that should be the default. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Mon, Aug 26, 2013 at 10:50 PM, Stephen Frost wrote: > Martijn, > > * Martijn van Oosterhout (klep...@svana.org) wrote: >> Note, my whole purpose for suggesting something like: >> >> include_auto_conf_filepostgresql.auto.conf >> >> is because I want the file location to be configurable. If I put in my >> configuration: >> >> include_auto_conf_file/etc/postgresql/9.4/postgresql.auto.conf > > I'm not following the use-case here. > > Why would it make sense for a file which is entirely managed by PG > automatically to be in /etc? Of course, by the same token you might ask > why it makes sense to let the user pick it at all, which holds some > merit- but I liked your idea because then an admin doesn't have to go > looking for the file but instead knows where it is. Perhaps comments in > the file stating where the auto.conf lives would be sufficient, but > those are too often nuked. :( > >> it better put the options there. And if I comment the line out ALTER >> SYSTEM should stop working. If I put it at the beginning of the config >> then any other option in the file will override it (which we can detect >> and warn about). If I put it at the end of the file, ALTER SYSTEM >> overrides any statically configured options. >> >> And I can imagine hosting providers putting the configuration for >> memory usage, shared library directories and other such options after, >> and options like cpu_cost, enable_merge_join, etc before. That way >> they have fine grained control over which options the user can set and >> which not. > > For my part, I'd honestly rather have the first things found be what's > picked and later things be ignored. If you want it controlled by ALTER > SYSTEM, then don't set it in postgresql.conf. The problem with that is > there's no "bootstrap" mechanism without actually modifying such configs > or making the configs be in auto.conf to begin with, neither of which is > ideal, imv. > > I really hate the idea that someone could configure 'X' in > postgresql.conf and because the auto.conf line is later in the file, > it's able to override the original setting. Does not strike me as > intuitive at all. This is currently how include mechanism works in postgresql.conf, changing that for this special case can be costly and moreover the specs for this patch were layout from beginning that way. However, we can have some mechanism for appending values (if feasible) as suggested in mail below, after initial patch is done http://www.postgresql.org/message-id/52015414.9060...@cybertec.at With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Martijn, * Martijn van Oosterhout (klep...@svana.org) wrote: > Note, my whole purpose for suggesting something like: > > include_auto_conf_filepostgresql.auto.conf > > is because I want the file location to be configurable. If I put in my > configuration: > > include_auto_conf_file/etc/postgresql/9.4/postgresql.auto.conf I'm not following the use-case here. Why would it make sense for a file which is entirely managed by PG automatically to be in /etc? Of course, by the same token you might ask why it makes sense to let the user pick it at all, which holds some merit- but I liked your idea because then an admin doesn't have to go looking for the file but instead knows where it is. Perhaps comments in the file stating where the auto.conf lives would be sufficient, but those are too often nuked. :( > it better put the options there. And if I comment the line out ALTER > SYSTEM should stop working. If I put it at the beginning of the config > then any other option in the file will override it (which we can detect > and warn about). If I put it at the end of the file, ALTER SYSTEM > overrides any statically configured options. > > And I can imagine hosting providers putting the configuration for > memory usage, shared library directories and other such options after, > and options like cpu_cost, enable_merge_join, etc before. That way > they have fine grained control over which options the user can set and > which not. For my part, I'd honestly rather have the first things found be what's picked and later things be ignored. If you want it controlled by ALTER SYSTEM, then don't set it in postgresql.conf. The problem with that is there's no "bootstrap" mechanism without actually modifying such configs or making the configs be in auto.conf to begin with, neither of which is ideal, imv. I really hate the idea that someone could configure 'X' in postgresql.conf and because the auto.conf line is later in the file, it's able to override the original setting. Does not strike me as intuitive at all. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Sat, Aug 24, 2013 at 6:08 PM, Martijn van Oosterhout wrote: > On Fri, Aug 23, 2013 at 06:41:04PM +0530, Amit Kapila wrote: >> > Not with this proposal... If it's fixed then it makes no sense to make it >> > look like it can be modified. >> >>This proposal is to have a special include which user can only use >> for enable/disable >>which means he can remove symbol '#' or add '#'. >>We cannot stop user from changing file name or add some different >> location, but that can lead to problems. >>We can have a note on top of this include indicating it is only for >> enable/disable. > > Note, my whole purpose for suggesting something like: > > include_auto_conf_filepostgresql.auto.conf > > is because I want the file location to be configurable. If I put in my > configuration: > > include_auto_conf_file/etc/postgresql/9.4/postgresql.auto.conf > > it better put the options there. And if I comment the line out ALTER > SYSTEM should stop working. If I put it at the beginning of the config > then any other option in the file will override it (which we can detect > and warn about). If I put it at the end of the file, ALTER SYSTEM > overrides any statically configured options. > > And I can imagine hosting providers putting the configuration for > memory usage, shared library directories and other such options after, > and options like cpu_cost, enable_merge_join, etc before. That way > they have fine grained control over which options the user can set and > which not. Thanks for your suggestion. Above usecase can be achieved even if the file is not configurable. >>I think if we add more meaning to it, like allow user to change it, >> handling and defining of that will be bit complex. > > Letting the user configure the location seems like common curtesy. Note > this line isn't in itself a GUC, so you can't configure it via ALTER > SYSTEM. In general yes it is better to give control to user for configuring the location, but as this file will be used for internal purpose and will be modified by system and not by user, so it is better to be in data directory. The similar thing was discussed previously on this thread and it is suggested to have this file in data directory. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Aug 23, 2013 at 06:41:04PM +0530, Amit Kapila wrote: > > Not with this proposal... If it's fixed then it makes no sense to make it > > look like it can be modified. > >This proposal is to have a special include which user can only use > for enable/disable >which means he can remove symbol '#' or add '#'. >We cannot stop user from changing file name or add some different > location, but that can lead to problems. >We can have a note on top of this include indicating it is only for > enable/disable. Note, my whole purpose for suggesting something like: include_auto_conf_filepostgresql.auto.conf is because I want the file location to be configurable. If I put in my configuration: include_auto_conf_file/etc/postgresql/9.4/postgresql.auto.conf it better put the options there. And if I comment the line out ALTER SYSTEM should stop working. If I put it at the beginning of the config then any other option in the file will override it (which we can detect and warn about). If I put it at the end of the file, ALTER SYSTEM overrides any statically configured options. And I can imagine hosting providers putting the configuration for memory usage, shared library directories and other such options after, and options like cpu_cost, enable_merge_join, etc before. That way they have fine grained control over which options the user can set and which not. >I think if we add more meaning to it, like allow user to change it, > handling and defining of that will be bit complex. Letting the user configure the location seems like common curtesy. Note this line isn't in itself a GUC, so you can't configure it via ALTER SYSTEM. Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Fri, Aug 23, 2013 at 6:01 PM, Stephen Frost wrote: > * Amit Kapila (amit.kapil...@gmail.com) wrote: >> On Thu, Aug 22, 2013 at 6:06 PM, Stephen Frost wrote: >> > * Amit Kapila (amit.kapil...@gmail.com) wrote: >> >> Enable/Disable reading of auto file >> >> - >> >> a. Have a new include in postresql.conf >> >> #include_auto_conf_filepostgresql.auto.conf >> >> as it is a special include, we can read this file relative to data >> >> directory. >> >> >> >> Enable/Disable Alter System command >> >> --- >> >> This can be achieved in 3 ways: >> >> a. Check before executing Alter System if include directive is >> >> disabled, then just issue a warning to user and proceed with command. >> >> b. Check before executing Alter System if include directive is >> >> disabled, then just issue an error and stop. >> > >> > It doesn't make sense for it to be a 'warning' with this- the >> > parameter specifies the file to use. If you don't know what file to >> > use, how you can possibly do anything but return an error? >> >>As the file and location are fixed, we can go-ahead and write to >> it, but I think now we are deciding >>if someone disables include dir, then we can just disable Alter >> System, so it is better to return error in such >>situation. > > It wouldn't be fixed with this approach. > >> > Note that I *like* that about this approach. >> > >> > There are a few other considerations with this- >> > >> > - What should the default be? (Still thinking 'off' myself) >> default 'off' is a safe option, as it won't allow users to make >> any change to parameter values until/unless they >> read from manual, how to use it and what can go wrong, on the >> other side it will be bit hassle for user to use this >> command. I think 'on' would be better. > > Yeah, no, I still think 'off' would be best for this particular option. > >> > - What happens if the user specifies 'postgresql.conf'? I'm thinking we >> > would disallow such insanity (as that's what it is, imv..) by having >> > an identifier in the file that this is the PG "auto conf" file. >> I think we can detect by name and give error. >> > - Should we have such an identifier in auto.conf to indicate that we >> > created it, to prevent the user from setting it to something they >> > shouldn't? >> I think if user plays with this file manually, it can lead to >> problems, that's why earlier we have >> decided to keep a note on top of file which will indicate, do not >> edit this file manually. >> I believe that should be sufficient. > > I agree that having such a disclaimer at the top of the file is a good > idea. I'm not completely convinced that's sufficient but it's certainly > better than nothing. > >> > - What's the "bootstrap" mode; iow, if a user enables the option but the >> > file doesn't exist, what do we do? With this approach, I'd be >> > inclined to say we simply create it and put the marker to indicate >> > it's "our" file. >> >> Alter System will create the file if doesn't exist. > > ... Only if it's enabled though. Yes. > >> > - Should we allow it to be outside of the data dir? We could simply log >> > an error and ignore the parameter if it's more than a simple filename. >> >> This should be an error, the file location and name will be fixed. > > Not with this proposal... If it's fixed then it makes no sense to make it > look like it can be modified. This proposal is to have a special include which user can only use for enable/disable which means he can remove symbol '#' or add '#'. We cannot stop user from changing file name or add some different location, but that can lead to problems. We can have a note on top of this include indicating it is only for enable/disable. I think if we add more meaning to it, like allow user to change it, handling and defining of that will be bit complex. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kapil...@gmail.com) wrote: > On Thu, Aug 22, 2013 at 6:06 PM, Stephen Frost wrote: > > * Amit Kapila (amit.kapil...@gmail.com) wrote: > >> Enable/Disable reading of auto file > >> - > >> a. Have a new include in postresql.conf > >> #include_auto_conf_filepostgresql.auto.conf > >> as it is a special include, we can read this file relative to data > >> directory. > >> > >> Enable/Disable Alter System command > >> --- > >> This can be achieved in 3 ways: > >> a. Check before executing Alter System if include directive is > >> disabled, then just issue a warning to user and proceed with command. > >> b. Check before executing Alter System if include directive is > >> disabled, then just issue an error and stop. > > > > It doesn't make sense for it to be a 'warning' with this- the > > parameter specifies the file to use. If you don't know what file to > > use, how you can possibly do anything but return an error? > >As the file and location are fixed, we can go-ahead and write to > it, but I think now we are deciding >if someone disables include dir, then we can just disable Alter > System, so it is better to return error in such >situation. It wouldn't be fixed with this approach. > > Note that I *like* that about this approach. > > > > There are a few other considerations with this- > > > > - What should the default be? (Still thinking 'off' myself) > default 'off' is a safe option, as it won't allow users to make > any change to parameter values until/unless they > read from manual, how to use it and what can go wrong, on the > other side it will be bit hassle for user to use this > command. I think 'on' would be better. Yeah, no, I still think 'off' would be best for this particular option. > > - What happens if the user specifies 'postgresql.conf'? I'm thinking we > > would disallow such insanity (as that's what it is, imv..) by having > > an identifier in the file that this is the PG "auto conf" file. > I think we can detect by name and give error. > > - Should we have such an identifier in auto.conf to indicate that we > > created it, to prevent the user from setting it to something they > > shouldn't? > I think if user plays with this file manually, it can lead to > problems, that's why earlier we have > decided to keep a note on top of file which will indicate, do not > edit this file manually. > I believe that should be sufficient. I agree that having such a disclaimer at the top of the file is a good idea. I'm not completely convinced that's sufficient but it's certainly better than nothing. > > - What's the "bootstrap" mode; iow, if a user enables the option but the > > file doesn't exist, what do we do? With this approach, I'd be > > inclined to say we simply create it and put the marker to indicate > > it's "our" file. > > Alter System will create the file if doesn't exist. ... Only if it's enabled though. > > - Should we allow it to be outside of the data dir? We could simply log > > an error and ignore the parameter if it's more than a simple filename. > > This should be an error, the file location and name will be fixed. Not with this proposal... If it's fixed then it makes no sense to make it look like it can be modified. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 22, 2013 at 9:34 PM, Bruce Momjian wrote: > On Thu, Aug 22, 2013 at 08:36:37AM -0400, Stephen Frost wrote: >> * Amit Kapila (amit.kapil...@gmail.com) wrote: >> >This can resolve the problem of whether to read auto file rather >> > cleanly, so the idea is: >> > >> > Enable/Disable reading of auto file >> > - >> > a. Have a new include in postresql.conf >> > #include_auto_conf_filepostgresql.auto.conf >> > as it is a special include, we can read this file relative to data >> > directory. > > The big advantage of using 'include_auto_conf_file' and not simply > 'include' is that we can issue an error from ALTER SYSTEM SET if that is > not set. > >> > Enable/Disable Alter System command >> > --- >> > This can be achieved in 3 ways: >> > a. Check before executing Alter System if include directive is >> > disabled, then just issue a warning to user and proceed with command. >> > b. Check before executing Alter System if include directive is >> > disabled, then just issue an error and stop. >> >> It doesn't make sense for it to be a 'warning' with this- the >> parameter specifies the file to use. If you don't know what file to >> use, how you can possibly do anything but return an error? > > Agreed. No sense in allowing users to add things to the 'auto' file > when the auto file is inactive. > >> Note that I *like* that about this approach. >> >> There are a few other considerations with this- >> >> - What should the default be? (Still thinking 'off' myself) > > Probably, but we might need to wait until we have a final API for a > decision on that. > >> - What happens if the user specifies 'postgresql.conf'? I'm thinking we >> would disallow such insanity (as that's what it is, imv..) by having >> an identifier in the file that this is the PG "auto conf" file. > > I am thinking they can't include a value equal to 'config_file', which > is normally postgresql.conf. I am not a big fan of looking for special > text in files. This might be complex to check, though, because of path > changes --- we might just disallow the basement from matching the > basename of config_file. Right, I also think that as file and location are fixed, so it can be detected with name. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 22, 2013 at 6:06 PM, Stephen Frost wrote: > * Amit Kapila (amit.kapil...@gmail.com) wrote: >>This can resolve the problem of whether to read auto file rather >> cleanly, so the idea is: >> >> Enable/Disable reading of auto file >> - >> a. Have a new include in postresql.conf >> #include_auto_conf_filepostgresql.auto.conf >> as it is a special include, we can read this file relative to data >> directory. >> >> Enable/Disable Alter System command >> --- >> This can be achieved in 3 ways: >> a. Check before executing Alter System if include directive is >> disabled, then just issue a warning to user and proceed with command. >> b. Check before executing Alter System if include directive is >> disabled, then just issue an error and stop. > > It doesn't make sense for it to be a 'warning' with this- the > parameter specifies the file to use. If you don't know what file to > use, how you can possibly do anything but return an error? As the file and location are fixed, we can go-ahead and write to it, but I think now we are deciding if someone disables include dir, then we can just disable Alter System, so it is better to return error in such situation. > Note that I *like* that about this approach. > > There are a few other considerations with this- > > - What should the default be? (Still thinking 'off' myself) default 'off' is a safe option, as it won't allow users to make any change to parameter values until/unless they read from manual, how to use it and what can go wrong, on the other side it will be bit hassle for user to use this command. I think 'on' would be better. > - What happens if the user specifies 'postgresql.conf'? I'm thinking we > would disallow such insanity (as that's what it is, imv..) by having > an identifier in the file that this is the PG "auto conf" file. I think we can detect by name and give error. > - Should we have such an identifier in auto.conf to indicate that we > created it, to prevent the user from setting it to something they > shouldn't? I think if user plays with this file manually, it can lead to problems, that's why earlier we have decided to keep a note on top of file which will indicate, do not edit this file manually. I believe that should be sufficient. > - What's the "bootstrap" mode; iow, if a user enables the option but the > file doesn't exist, what do we do? With this approach, I'd be > inclined to say we simply create it and put the marker to indicate > it's "our" file. Alter System will create the file if doesn't exist. > - Should we allow it to be outside of the data dir? We could simply log > an error and ignore the parameter if it's more than a simple filename. This should be an error, the file location and name will be fixed. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Thu, Aug 22, 2013 at 08:36:37AM -0400, Stephen Frost wrote: > * Amit Kapila (amit.kapil...@gmail.com) wrote: > >This can resolve the problem of whether to read auto file rather > > cleanly, so the idea is: > > > > Enable/Disable reading of auto file > > - > > a. Have a new include in postresql.conf > > #include_auto_conf_filepostgresql.auto.conf > > as it is a special include, we can read this file relative to data > > directory. The big advantage of using 'include_auto_conf_file' and not simply 'include' is that we can issue an error from ALTER SYSTEM SET if that is not set. > > Enable/Disable Alter System command > > --- > > This can be achieved in 3 ways: > > a. Check before executing Alter System if include directive is > > disabled, then just issue a warning to user and proceed with command. > > b. Check before executing Alter System if include directive is > > disabled, then just issue an error and stop. > > It doesn't make sense for it to be a 'warning' with this- the > parameter specifies the file to use. If you don't know what file to > use, how you can possibly do anything but return an error? Agreed. No sense in allowing users to add things to the 'auto' file when the auto file is inactive. > Note that I *like* that about this approach. > > There are a few other considerations with this- > > - What should the default be? (Still thinking 'off' myself) Probably, but we might need to wait until we have a final API for a decision on that. > - What happens if the user specifies 'postgresql.conf'? I'm thinking we > would disallow such insanity (as that's what it is, imv..) by having > an identifier in the file that this is the PG "auto conf" file. I am thinking they can't include a value equal to 'config_file', which is normally postgresql.conf. I am not a big fan of looking for special text in files. This might be complex to check, though, because of path changes --- we might just disallow the basement from matching the basename of config_file. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kapil...@gmail.com) wrote: >This can resolve the problem of whether to read auto file rather > cleanly, so the idea is: > > Enable/Disable reading of auto file > - > a. Have a new include in postresql.conf > #include_auto_conf_filepostgresql.auto.conf > as it is a special include, we can read this file relative to data > directory. > > Enable/Disable Alter System command > --- > This can be achieved in 3 ways: > a. Check before executing Alter System if include directive is > disabled, then just issue a warning to user and proceed with command. > b. Check before executing Alter System if include directive is > disabled, then just issue an error and stop. It doesn't make sense for it to be a 'warning' with this- the parameter specifies the file to use. If you don't know what file to use, how you can possibly do anything but return an error? Note that I *like* that about this approach. There are a few other considerations with this- - What should the default be? (Still thinking 'off' myself) - What happens if the user specifies 'postgresql.conf'? I'm thinking we would disallow such insanity (as that's what it is, imv..) by having an identifier in the file that this is the PG "auto conf" file. - Should we have such an identifier in auto.conf to indicate that we created it, to prevent the user from setting it to something they shouldn't? - What's the "bootstrap" mode; iow, if a user enables the option but the file doesn't exist, what do we do? With this approach, I'd be inclined to say we simply create it and put the marker to indicate it's "our" file. - Should we allow it to be outside of the data dir? We could simply log an error and ignore the parameter if it's more than a simple filename. There are probably other considerations also.. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Aug 21, 2013 at 8:22 PM, Stephen Frost wrote: > Martijn, > > * Martijn van Oosterhout (klep...@svana.org) wrote: >> ISTM you want some kind of hybrid setting like: >> >> #include_system auto.conf >> >> which simultaneously does three things: >> >> 1. Sets the enable_alter_system flag >> 2. Indicates the file to use >> 3. Indicates the priority of the setting re other settings. >> >> Comment it out, ALTER SYSTEM stop working. Put it back and it's >> immediately clear what it means. And the user can control where the >> settings go. > > Yeah, that's certainly an interesting idea. I might call it > 'auto_conf_file auto.conf' to avoid the '#include' concern and to > perhaps clarify that it's more than just a regular 'include'. This can resolve the problem of whether to read auto file rather cleanly, so the idea is: Enable/Disable reading of auto file - a. Have a new include in postresql.conf #include_auto_conf_filepostgresql.auto.conf as it is a special include, we can read this file relative to data directory. Enable/Disable Alter System command --- This can be achieved in 3 ways: a. Check before executing Alter System if include directive is disabled, then just issue a warning to user and proceed with command. b. Check before executing Alter System if include directive is disabled, then just issue an error and stop. c. Have a new guc enable_alter_system which will behave as described in my previous mail and below: >1. enable_alter_system a new GUC whose default value =off. > 2. Alter System will check this variable and return error (not > allowed), if this parameter is off. > 3. Now if user enables include directive in postgresql.conf, it will > enable Alter System as value of enable_alter_system is on. > 4. User can run Alter System command to disable Alter System > "enable_alter_system = off". > Now even though include directive is enabled, but new Alter System > commands will not work, however > existing parameter's take into effect on restart/sighup. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Martijn, * Martijn van Oosterhout (klep...@svana.org) wrote: > ISTM you want some kind of hybrid setting like: > > #include_system auto.conf > > which simultaneously does three things: > > 1. Sets the enable_alter_system flag > 2. Indicates the file to use > 3. Indicates the priority of the setting re other settings. > > Comment it out, ALTER SYSTEM stop working. Put it back and it's > immediately clear what it means. And the user can control where the > settings go. Yeah, that's certainly an interesting idea. I might call it 'auto_conf_file auto.conf' to avoid the '#include' concern and to perhaps clarify that it's more than just a regular 'include'. > Syntax is a bit fugly though. Agreed. Thanks, Stephen (who is still unhappy about the GUC-specific handling for relative paths in postgresql.conf) signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Wed, Aug 21, 2013 at 7:29 AM, Bruce Momjian wrote: > On Tue, Aug 20, 2013 at 08:38:52AM +0530, Amit Kapila wrote: >> On Tue, Aug 20, 2013 at 8:27 AM, Stephen Frost wrote: >> > * Amit Kapila (amit.kapil...@gmail.com) wrote: >> >> If both are okay, then I would like to go with second option (include >> >> file mechanism). >> >> I think by default, we should allow auto file to be included and if >> >> user faces any problem or otherwise, >> >> then he can decide to disable it. >> > >> > If it's enabled by default, then we need to ship an 'auto' file which is >> > empty by default... >> >> initdb will create the empty auto file. If we don't enable by default, >> then if user uses >> ALTER SYSTEM and do sighup/restart, changed config parameter values >> will not come into affect >> until he manually enables it by removing '#' from '#include'. > > Just a heads-up, but a lot of C language folks are going to look at: > > #include abc > > and think that is enabled. True, but generally for conf and script file, symbol '#' is treated as commented portion of content. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 20, 2013 at 10:02 PM, Alvaro Herrera wrote: > Stephen Frost escribió: >> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: >> > Well, all the relative paths in include/includedir directives would be >> > relative to the directory specified by the -c config_file param, which >> > makes perfect sense. So the conf.d would work fine in your example. >> >> Why would include/includedir directives be relative to where the >> 'config_file' GUC is set to instead of relative to where all the other >> GUCs in postgresql.conf are relative to? That is a recipe for >> confusion, imv. >> >> Of course, the current situation is quite terrible anyway, imv. Having >> the GUCs be relative to whereever the user happens to run pg_ctl from is >> pretty ugly- not to mention that the commented out 'defaults' won't >> actually work if you uncomment them because the *actual* default/unset >> value *is* in the data directory. > > Uh? See the docs: > http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES > > " ... the postgresql.conf file can contain include directives, ... > If the file name is not an absolute path, it is taken as relative to > the directory containing the referencing configuration file." You are right and I have checked code as well, it works in above way for include directives. Now the question I have in mind is that even if we can't directly use include directive to enable/disable Alter System and reading of auto file, how would a new GUC enable_alter_system can solve all the things. It can allow/disallow Alter System command, but how about reading of auto file? If we directly read auto file without considering enable_alter_system, it can lead to chaos due to some un-safe values, on the other side if we want to consider enable_alter_system before reading file, it can complicate the ProcessConfigFile() code. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 20, 2013 at 08:38:52AM +0530, Amit Kapila wrote: > On Tue, Aug 20, 2013 at 8:27 AM, Stephen Frost wrote: > > * Amit Kapila (amit.kapil...@gmail.com) wrote: > >> If both are okay, then I would like to go with second option (include > >> file mechanism). > >> I think by default, we should allow auto file to be included and if > >> user faces any problem or otherwise, > >> then he can decide to disable it. > > > > If it's enabled by default, then we need to ship an 'auto' file which is > > empty by default... > > initdb will create the empty auto file. If we don't enable by default, > then if user uses > ALTER SYSTEM and do sighup/restart, changed config parameter values > will not come into affect > until he manually enables it by removing '#' from '#include'. Just a heads-up, but a lot of C language folks are going to look at: #include abc and think that is enabled. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 20, 2013 at 11:23:06AM -0400, Stephen Frost wrote: > > What I was proposing upthread is that enable_alter_system=off/on would > > be present in postgresql.conf, and there is no include line for > > auto.conf. > > I really think that's a terrible approach, to be honest. I want to see > an 'include' line in postgresql.conf for auto.conf, so the hapless > sysadmin who is trying to figure out what the crazy DBA did has some > clue what to look for. "enable_alter_system" doesn't tell him diddly > about an 'auto.conf' file which is included in the system config. ISTM you want some kind of hybrid setting like: #include_system auto.conf which simultaneously does three things: 1. Sets the enable_alter_system flag 2. Indicates the file to use 3. Indicates the priority of the setting re other settings. Comment it out, ALTER SYSTEM stop working. Put it back and it's immediately clear what it means. And the user can control where the settings go. Syntax is a bit fugly though. Have a nice day, -- Martijn van Oosterhout http://svana.org/kleptog/ > He who writes carelessly confesses thereby at the very outset that he does > not attach much importance to his own thoughts. -- Arthur Schopenhauer signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost escribió: > > http://www.postgresql.org/docs/9.3/static/runtime-config-file-locations.html > > That's talking about the data_directory and the various foo_file > settings, though; it doesn't apply to the include settings. Right- that's what I'm bitching about. We have various references to file locations, with defined handling of relative locations, and the 'include' system completely ignores that and instead invents its own making the result a confusing mess. > Note > especially that config_file says it can only be set on the postgres > command line. (I think a saner definition would have been to state that > relative paths are not allowed in the command line. But that ship has > already sailed. And relative paths seem useful in the config file; and > maintaining the distinction that they are allowed within the config file > but not in the command line might be awkward.) Relative paths based on $PWD are useful? Really? Not on my systems anyway.. > (Uhm, when a command line contains stuff, is the stuff "in" the command > line or "on" it?) A just question- I vote 'on'. :) Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > And what about > > http://www.postgresql.org/docs/9.3/static/runtime-config-file-locations.html > > ... ? > >"When setting any of these parameters, a relative path will be >interpreted with respect to the directory in which postgres is >started." That's talking about the data_directory and the various foo_file settings, though; it doesn't apply to the include settings. Note especially that config_file says it can only be set on the postgres command line. (I think a saner definition would have been to state that relative paths are not allowed in the command line. But that ship has already sailed. And relative paths seem useful in the config file; and maintaining the distinction that they are allowed within the config file but not in the command line might be awkward.) (Uhm, when a command line contains stuff, is the stuff "in" the command line or "on" it?) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost escribió: > > Of course, the current situation is quite terrible anyway, imv. Having > > the GUCs be relative to whereever the user happens to run pg_ctl from is > > pretty ugly- not to mention that the commented out 'defaults' won't > > actually work if you uncomment them because the *actual* default/unset > > value *is* in the data directory. > > Uh? See the docs: > http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES > > " ... the postgresql.conf file can contain include directives, ... > If the file name is not an absolute path, it is taken as relative to > the directory containing the referencing configuration file." And what about http://www.postgresql.org/docs/9.3/static/runtime-config-file-locations.html ... ? "When setting any of these parameters, a relative path will be interpreted with respect to the directory in which postgres is started." > > I'm starting to wish for a way to do > > variable substitution inside postgresql.conf, so we could have defaults > > that would actually work if uncommented (eg: $DataDir/pg_hba.conf). > > > > That would help with auto.conf also. > > Grumble. I don't think we need this. At least, not for ALTER SYSTEM or > conf.d. imv, it would be handy. Along with a $ConfDir which is the postgresql.conf location- it would certainly make things clearer about what files are being included from where. > > To be honest, I was considering 'include' to be relative to the data > > directory and handled similar to how we process recovery.conf, > > Well, recovery.conf is a special case that doesn't follow to normal > rules. I don't see why it should be. > > but as we > > consider paths in postgresql.conf to be relative to $PWD, that's not > > ideal because it'd be different from the rest of the file references. > > I don't know where you got that idea from, but it's wrong. Not sure which you're referring to, but see above from the docs? Also, please tias.. I was amazed that we use $PWD also, but we do.. > Well, this whole line of discussion started because I objected to the > whole code path that was trying to detect whether auto.conf had been > parsed, and raised a warning if ALTER SYSTEM was executed and the file > wasn't parsed. I like the idea that we complain if ALTER SYSTEM ends up being idempotent.. Not sure how far we should take that, but accepting commands which end up doing nothing is bad, imv. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Alvaro Herrera writes: > Stephen Frost escribió: >> While I really like the 'include auto.conf' style, I'm starting to think >> it may not be workable after all. Another thing to consider is if the >> user decides to change that include line.. What happens when the DBA >> tries to do a 'ALTER SYSTEM'? It'd still use the hard-coded auto.conf >> file and happily update it, I imagine, but it won't actually get >> included... > Well, this whole line of discussion started because I objected to the > whole code path that was trying to detect whether auto.conf had been > parsed, and raised a warning if ALTER SYSTEM was executed and the file > wasn't parsed. I really, really don't think that we should be trying to detect or prevent any such thing. If the user breaks it like that, he gets to keep both pieces --- and who's to say it's broken, anyway? Disabling ALTER SYSTEM might have been exactly his intent. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Well, all the relative paths in include/includedir directives would be > > relative to the directory specified by the -c config_file param, which > > makes perfect sense. So the conf.d would work fine in your example. > > Why would include/includedir directives be relative to where the > 'config_file' GUC is set to instead of relative to where all the other > GUCs in postgresql.conf are relative to? That is a recipe for > confusion, imv. > > Of course, the current situation is quite terrible anyway, imv. Having > the GUCs be relative to whereever the user happens to run pg_ctl from is > pretty ugly- not to mention that the commented out 'defaults' won't > actually work if you uncomment them because the *actual* default/unset > value *is* in the data directory. Uh? See the docs: http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES " ... the postgresql.conf file can contain include directives, ... If the file name is not an absolute path, it is taken as relative to the directory containing the referencing configuration file." > I'm starting to wish for a way to do > variable substitution inside postgresql.conf, so we could have defaults > that would actually work if uncommented (eg: $DataDir/pg_hba.conf). > > That would help with auto.conf also. Grumble. I don't think we need this. At least, not for ALTER SYSTEM or conf.d. > > The only problem I see in your snippet above is the "include auto.conf" > > line, which doesn't make any sense because that file would not be found. > > To be honest, I was considering 'include' to be relative to the data > directory and handled similar to how we process recovery.conf, Well, recovery.conf is a special case that doesn't follow to normal rules. > but as we > consider paths in postgresql.conf to be relative to $PWD, that's not > ideal because it'd be different from the rest of the file references. I don't know where you got that idea from, but it's wrong. > While I really like the 'include auto.conf' style, I'm starting to think > it may not be workable after all. Another thing to consider is if the > user decides to change that include line.. What happens when the DBA > tries to do a 'ALTER SYSTEM'? It'd still use the hard-coded auto.conf > file and happily update it, I imagine, but it won't actually get > included... Well, this whole line of discussion started because I objected to the whole code path that was trying to detect whether auto.conf had been parsed, and raised a warning if ALTER SYSTEM was executed and the file wasn't parsed. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Well, all the relative paths in include/includedir directives would be > relative to the directory specified by the -c config_file param, which > makes perfect sense. So the conf.d would work fine in your example. Why would include/includedir directives be relative to where the 'config_file' GUC is set to instead of relative to where all the other GUCs in postgresql.conf are relative to? That is a recipe for confusion, imv. Of course, the current situation is quite terrible anyway, imv. Having the GUCs be relative to whereever the user happens to run pg_ctl from is pretty ugly- not to mention that the commented out 'defaults' won't actually work if you uncomment them because the *actual* default/unset value *is* in the data directory. I'm starting to wish for a way to do variable substitution inside postgresql.conf, so we could have defaults that would actually work if uncommented (eg: $DataDir/pg_hba.conf). That would help with auto.conf also. > The only problem I see in your snippet above is the "include auto.conf" > line, which doesn't make any sense because that file would not be found. To be honest, I was considering 'include' to be relative to the data directory and handled similar to how we process recovery.conf, but as we consider paths in postgresql.conf to be relative to $PWD, that's not ideal because it'd be different from the rest of the file references. In any case, while the current situation sucks, I don't think we can go changing how relative files are treated in postgresql.conf, nor should we make the way a path is processed change depending on which GUC is being set. While I really like the 'include auto.conf' style, I'm starting to think it may not be workable after all. Another thing to consider is if the user decides to change that include line.. What happens when the DBA tries to do a 'ALTER SYSTEM'? It'd still use the hard-coded auto.conf file and happily update it, I imagine, but it won't actually get included... Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > Tried to in my other mail, Yep, got it and replied, thanks. > but let me also point out that we > ("PGDG"/Upstream) don't "own" the directory in which postgresql.conf > lives. At least on Debian and relatives, that directory isn't under > $PGDATA and it already has other files in it beyond postgresql.conf or > even the other PostgreSQL config files of hba and ident. Here's the > default directory setup on Debian for /etc/postgresql/9.2/main/: > > -rw-r--r-- 1 postgres postgres 316 Jun 29 22:07 environment > -rw-r--r-- 1 postgres postgres 143 Jun 29 22:07 pg_ctl.conf > -rw-r- 1 postgres postgres 4649 Jun 29 22:07 pg_hba.conf > -rw-r- 1 postgres postgres 1636 Jun 29 22:07 pg_ident.conf > -rw-r--r-- 1 postgres postgres 19770 Jun 29 22:07 postgresql.conf > -rw-r--r-- 1 postgres postgres 378 Jun 29 22:07 start.conf > > There's three other files there and some sysadmins may have already > created their own 'conf.d' directory, perhaps to use for building the > postgresql.conf file or similar. We must have a way to disable the > conf.d option and a way to name it something other than 'conf.d', imv. Uhm. I find it hard to care much for this position. Surely config files are not migrated automatically from one major version to the next, so if somebody created a 9.3/main/conf.d directory, they will have to change it when they migrate to 9.4. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > I agree that auto.conf should live in $PGDATA, but I really don't like > the idea of conf.d being relative to some other existing file. It > should be included through an 'includedir' option, not just picked up as > some magic directory name, and therefore consider the current > arrangement of parameters in Debian: > > data_directory = '/var/lib/postgresql/9.2/main' > hba_file = '/etc/postgresql/9.2/main/pg_hba.conf' > ident_file = '/etc/postgresql/9.2/main/pg_ident.conf' > > and postgres is started like so: > > /usr/lib/postgresql/9.2/bin/postgres -D /var/lib/postgresql/9.2/main -c > config_file=/etc/postgresql/9.2/main/postgresql.conf > > With the proposed include line for auto.conf, which lives in $PGDATA, > we'd have: > > include 'auto.conf' > > Would we then have > > includedir 'conf.d' > > which is relative to postgresql.conf instead? Well, all the relative paths in include/includedir directives would be relative to the directory specified by the -c config_file param, which makes perfect sense. So the conf.d would work fine in your example. The only problem I see in your snippet above is the "include auto.conf" line, which doesn't make any sense because that file would not be found. Hence my proposal that the file ought to be read automatically, not via an include line. Sadly I don't think we cannot just make it an absolute path, in case the data dir is moved or whatever; the only choice I see would be to have something like include-data 'auto.conf' or something like that which tells the system that the file is not in the config dir but in the data dir instead. A nearby comment could let the user know about this file being in the data directory instead of the config directory. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost escribió: > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > > Stephen Frost escribió: > > > > > I'd much rather have an includedir directive than some hard-coded or > > > > command-line option to read the directory.. The directory should live > > > > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives.. > > > > > > The conf.d/ path would be relative to postgresql.conf, so there's no > > > need for Debian to patch anything. > > > > Uhhh, I really don't see that working, at all... > > I don't understand why not. Care to explain? Tried to in my other mail, but let me also point out that we ("PGDG"/Upstream) don't "own" the directory in which postgresql.conf lives. At least on Debian and relatives, that directory isn't under $PGDATA and it already has other files in it beyond postgresql.conf or even the other PostgreSQL config files of hba and ident. Here's the default directory setup on Debian for /etc/postgresql/9.2/main/: -rw-r--r-- 1 postgres postgres 316 Jun 29 22:07 environment -rw-r--r-- 1 postgres postgres 143 Jun 29 22:07 pg_ctl.conf -rw-r- 1 postgres postgres 4649 Jun 29 22:07 pg_hba.conf -rw-r- 1 postgres postgres 1636 Jun 29 22:07 pg_ident.conf -rw-r--r-- 1 postgres postgres 19770 Jun 29 22:07 postgresql.conf -rw-r--r-- 1 postgres postgres 378 Jun 29 22:07 start.conf There's three other files there and some sysadmins may have already created their own 'conf.d' directory, perhaps to use for building the postgresql.conf file or similar. We must have a way to disable the conf.d option and a way to name it something other than 'conf.d', imv. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Stephen Frost escribió: > > > I'd much rather have an includedir directive than some hard-coded or > > > command-line option to read the directory.. The directory should live > > > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives.. > > > > The conf.d/ path would be relative to postgresql.conf, so there's no > > need for Debian to patch anything. > > Uhhh, I really don't see that working, at all... I don't understand why not. Care to explain? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > >> The conf.d/ path would be relative to postgresql.conf, so there's no > >> need for Debian to patch anything. > > > Uhhh, I really don't see that working, at all... > > Why not? conf.d is for installable files, AIUI. What we need to > be writable is auto.conf, but I thought we'd agreed that would live > inside $PGDATA. I agree that auto.conf should live in $PGDATA, but I really don't like the idea of conf.d being relative to some other existing file. It should be included through an 'includedir' option, not just picked up as some magic directory name, and therefore consider the current arrangement of parameters in Debian: data_directory = '/var/lib/postgresql/9.2/main' hba_file = '/etc/postgresql/9.2/main/pg_hba.conf' ident_file = '/etc/postgresql/9.2/main/pg_ident.conf' and postgres is started like so: /usr/lib/postgresql/9.2/bin/postgres -D /var/lib/postgresql/9.2/main -c config_file=/etc/postgresql/9.2/main/postgresql.conf With the proposed include line for auto.conf, which lives in $PGDATA, we'd have: include 'auto.conf' Would we then have includedir 'conf.d' which is relative to postgresql.conf instead? Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost writes: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: >> The conf.d/ path would be relative to postgresql.conf, so there's no >> need for Debian to patch anything. > Uhhh, I really don't see that working, at all... Why not? conf.d is for installable files, AIUI. What we need to be writable is auto.conf, but I thought we'd agreed that would live inside $PGDATA. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost escribió: > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > > Stephen Frost escribió: > > > > With includedir and include directives, and postgresql.conf setting a > > > > defined ordering, with last-wins, you could simply have the 'includedir' > > > > for conf.d come before the 'include' for auto.conf. > > > > > > Uh, I was thinking that conf.d would be read by the system > > > automatically, not because of an includedir line in postgresql.conf. > > > > I'd much rather have an includedir directive than some hard-coded or > > command-line option to read the directory.. The directory should live > > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives.. > > The conf.d/ path would be relative to postgresql.conf, so there's no > need for Debian to patch anything. Uhhh, I really don't see that working, at all... Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > With this design, if you put enable_alter_system=off in auto.conf, there > is no way for the user to enable alter system again short of editing a > file in the data directory. I think this is one of the things that was > "forbidden" by policy; only files in the config directory needs to be > edited. If you edit it by hand to begin with (setting that parameter to 'off') then it's reasonable that you may have to edit it by hand again to fix it. If we don't want people to "lock themselves out" by using ALTER SYSTEM to turn it off, then we just disallow that. > What I was proposing upthread is that enable_alter_system=off/on would > be present in postgresql.conf, and there is no include line for > auto.conf. I really think that's a terrible approach, to be honest. I want to see an 'include' line in postgresql.conf for auto.conf, so the hapless sysadmin who is trying to figure out what the crazy DBA did has some clue what to look for. "enable_alter_system" doesn't tell him diddly about an 'auto.conf' file which is included in the system config. > That way, if the user wishes to enable/disable the feature, > they need to edit postgresql.conf to do so. ALTER SYSTEM doesn't offer > a way to disable itself. We can simply disallow ALTER SYSTEM from modifying enable_alter_system; that strikes me as a reasonable thing to do anyway. What I find a bit more worrying is what happens if they decide to put enable_alter_system=off into the postgresql.conf but keep the 'include' line for auto.conf.. Which goes right back to the question that I had before around if we want to complain when the same GUC is seen multiple times during parsing. It seems like there's no hope for it, given the way this has been designed, because you *must* set certain parameters and so you can't simply have them commented out, but those are likely to be parameters which DBAs will want to change through ALTER SYSTEM. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Alvaro Herrera writes: > What I was proposing upthread is that enable_alter_system=off/on would > be present in postgresql.conf, and there is no include line for > auto.conf. That way, if the user wishes to enable/disable the feature, > they need to edit postgresql.conf to do so. ALTER SYSTEM doesn't offer > a way to disable itself. If ALTER SYSTEM is disabled via > enable_alter_system=off in postgresql.conf, the settings in auto.conf > are not read. Personally, I'd get rid of any explicit "enable_alter_system" variable, and instead have an include directive for auto.conf. The functionality you describe above is then available by commenting or uncommenting the directive, plus the user has the option to decide where to put the directive (and thus control which settings can or can't be overridden). Anything else seems like it's going to be less flexible or else a lot more complicated. 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Stephen Frost escribió: > > > With includedir and include directives, and postgresql.conf setting a > > > defined ordering, with last-wins, you could simply have the 'includedir' > > > for conf.d come before the 'include' for auto.conf. > > > > Uh, I was thinking that conf.d would be read by the system > > automatically, not because of an includedir line in postgresql.conf. > > I'd much rather have an includedir directive than some hard-coded or > command-line option to read the directory.. The directory should live > in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives.. The conf.d/ path would be relative to postgresql.conf, so there's no need for Debian to patch anything. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > * Amit Kapila (amit.kapil...@gmail.com) wrote: > > So let me try to explain what I understood from above: > > > > 1. enable_alter_system a new GUC whose default value =off. > > 2. Alter System will check this variable and return error (not > > allowed), if this parameter is off. > > 3. Now if user enables include directive in postgresql.conf, it will > > enable Alter System as value of > > enable_alter_system is on. > > 4. User can run Alter System command to disable Alter System > > "enable_alter_system = off". > > Now even though include directive is enabled, but new Alter System > > commands will not work, however > > existing parameter's take into effect on restart/sighup. > > Yes. Not sure that it'd be terribly likely for a user to do that, but > if they do it, so be it. With this design, if you put enable_alter_system=off in auto.conf, there is no way for the user to enable alter system again short of editing a file in the data directory. I think this is one of the things that was "forbidden" by policy; only files in the config directory needs to be edited. What I was proposing upthread is that enable_alter_system=off/on would be present in postgresql.conf, and there is no include line for auto.conf. That way, if the user wishes to enable/disable the feature, they need to edit postgresql.conf to do so. ALTER SYSTEM doesn't offer a way to disable itself. If ALTER SYSTEM is disabled via enable_alter_system=off in postgresql.conf, the settings in auto.conf are not read. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Stephen Frost escribió: > > With includedir and include directives, and postgresql.conf setting a > > defined ordering, with last-wins, you could simply have the 'includedir' > > for conf.d come before the 'include' for auto.conf. > > Uh, I was thinking that conf.d would be read by the system > automatically, not because of an includedir line in postgresql.conf. I'd much rather have an includedir directive than some hard-coded or command-line option to read the directory.. The directory should live in /etc/postgresql/X.Y/cluster/ on at least Debian derivatives.. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
Stephen Frost escribió: > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > > Hopefully snippets put in conf.d/ by > > puppet/chef will override the settings in postgresql.conf (i.e. conf.d/ > > should be processed after postgresql.conf, not before); and hopefully > > ALTER SYSTEM will in turn override conf.d. I see no way to have ALTER > > SYSTEM handled by an include line, yet still have it override conf.d. > > With includedir and include directives, and postgresql.conf setting a > defined ordering, with last-wins, you could simply have the 'includedir' > for conf.d come before the 'include' for auto.conf. Uh, I was thinking that conf.d would be read by the system automatically, not because of an includedir line in postgresql.conf. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kapil...@gmail.com) wrote: > So let me try to explain what I understood from above: > > 1. enable_alter_system a new GUC whose default value =off. > 2. Alter System will check this variable and return error (not > allowed), if this parameter is off. > 3. Now if user enables include directive in postgresql.conf, it will > enable Alter System as value of > enable_alter_system is on. > 4. User can run Alter System command to disable Alter System > "enable_alter_system = off". > Now even though include directive is enabled, but new Alter System > commands will not work, however > existing parameter's take into effect on restart/sighup. Yes. Not sure that it'd be terribly likely for a user to do that, but if they do it, so be it. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 20, 2013 at 7:51 PM, Stephen Frost wrote: > * Amit Kapila (amit.kapil...@gmail.com) wrote: >> Wouldn't it be complicated to handle this way rather then by having >> include directive. > > You misunderstood. I was suggesting a setup along these lines: > > postgresql.conf: > # include = 'auto.conf' # Disabled by default > > auto.conf: > > # MANAGED BY ALTER SYSTEM -- DO NOT MODIFY BY HAND > > > # auto.conf not processed unless included by postgresql.conf > # If included by postgresql.conf, then this will turn on the > # ALTER SYSTEM command. > enable_alter_system = on > > Which would give us the ability to independently turn on/off ALTER > SYSTEM from including auto.conf. So let me try to explain what I understood from above: 1. enable_alter_system a new GUC whose default value =off. 2. Alter System will check this variable and return error (not allowed), if this parameter is off. 3. Now if user enables include directive in postgresql.conf, it will enable Alter System as value of enable_alter_system is on. 4. User can run Alter System command to disable Alter System "enable_alter_system = off". Now even though include directive is enabled, but new Alter System commands will not work, however existing parameter's take into effect on restart/sighup. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kapil...@gmail.com) wrote: > Wouldn't it be complicated to handle this way rather then by having > include directive. You misunderstood. I was suggesting a setup along these lines: postgresql.conf: # include = 'auto.conf' # Disabled by default auto.conf: # MANAGED BY ALTER SYSTEM -- DO NOT MODIFY BY HAND # auto.conf not processed unless included by postgresql.conf # If included by postgresql.conf, then this will turn on the # ALTER SYSTEM command. enable_alter_system = on Which would give us the ability to independently turn on/off ALTER SYSTEM from including auto.conf. Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 20, 2013 at 6:55 PM, Stephen Frost wrote: > * Amit Kapila (amit.kapil...@gmail.com) wrote: >> On Tue, Aug 20, 2013 at 6:21 PM, Stephen Frost wrote: >> > I'm not particularly set on this.. Why not create the file initially? >>If by default this feature needs to be disabled, then it is not >> must to have at initdb time. > > I don't see how the two are related. You could never use two-phase > commit (could even disable it), yet we still create things in the data > directory to support it. Having a file in the data directory which > isn't used by default isn't that big a deal, imv. Yes, it can be done, what I wanted to say it is not a "must", rather a good to have thing. >>Also you mentioned below line upthread which I understood as you >> don't like idea of creating empty file at initdb >>time: >>"If it's enabled by default, then we need to ship an 'auto' file which is >> empty by default... I don't particularly like that" > > What I didn't like was having an empty file be accepted as 'valid', but > you need to have some kind of bootstrap mechanism. Telling users "run > this command and then ignore the warning" is certainly bad. A better > option might be to have a *non-empty* auto.conf be generated, where all > it has in it is some kind of identifier, perhaps even just > "enable_alter_system = on" which we could then key off of to see if > ALTER SYSTEM has been set up to be allowed or not. Wouldn't it be complicated to handle this way rather then by having include directive. If include directive is enabled then the autofile will be read else no need to read it. If we want to have separate identifier to judge ALTER SYSTEM is enable or not, then it is better to go with a new GUC. >> I think if we choose to use include directive as a way to >> enable/disable this feature, it will not be good to allow change of >> this parameter with Alter System. > > I agree, but then we need to add it to the list of things ALTER SYSTEM > can't modify (if the include is implemented as a GUC, that is; I've not > looked). I have checked and it seems to be just parse time thing. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
* Amit Kapila (amit.kapil...@gmail.com) wrote: > On Tue, Aug 20, 2013 at 6:21 PM, Stephen Frost wrote: > > I'm not particularly set on this.. Why not create the file initially? >If by default this feature needs to be disabled, then it is not > must to have at initdb time. I don't see how the two are related. You could never use two-phase commit (could even disable it), yet we still create things in the data directory to support it. Having a file in the data directory which isn't used by default isn't that big a deal, imv. >Also you mentioned below line upthread which I understood as you > don't like idea of creating empty file at initdb >time: >"If it's enabled by default, then we need to ship an 'auto' file which is > empty by default... I don't particularly like that" What I didn't like was having an empty file be accepted as 'valid', but you need to have some kind of bootstrap mechanism. Telling users "run this command and then ignore the warning" is certainly bad. A better option might be to have a *non-empty* auto.conf be generated, where all it has in it is some kind of identifier, perhaps even just "enable_alter_system = on" which we could then key off of to see if ALTER SYSTEM has been set up to be allowed or not. > I think if we choose to use include directive as a way to > enable/disable this feature, it will not be good to allow change of > this parameter with Alter System. I agree, but then we need to add it to the list of things ALTER SYSTEM can't modify (if the include is implemented as a GUC, that is; I've not looked). Thanks, Stephen signature.asc Description: Digital signature
Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])
On Tue, Aug 20, 2013 at 6:21 PM, Stephen Frost wrote: > * Amit Kapila (amit.kapil...@gmail.com) wrote: >> Okay, let us decide how things will be if we disable by default: >>1. initdb won't create any empty auto file, rather the file will be >> created first time user uses ALTER SYSTEM. > > I'm not particularly set on this.. Why not create the file initially? If by default this feature needs to be disabled, then it is not must to have at initdb time. OTOH if we want to enable it by default, then we need to get this created at initdb time else it will give error during system startup (an error can occur during startup when it will parse postgresql.conf and didn't find the file mentioned in include directive). Also you mentioned below line upthread which I understood as you don't like idea of creating empty file at initdb time: "If it's enabled by default, then we need to ship an 'auto' file which is empty by default... I don't particularly like that" >>2. Alter system should issue warning, if it detects that feature is >> disabled (for this we need to error detection code >>during parsing of postgresql.conf as it was previously) > > I agree that it should complain through a warning or perhaps an error > (to be honest, I like error more, but there may be good reasons against > that). > >>3. postgresql.conf will contain include directive in below form: >>#include = 'postgresql.auto.conf' >>Whenever user wants to use Alter System, he needs to enable it >> after first time using ALTER SYSTEM. > > That I don't like.. You should be able to enable it and have things > "work" without having to run ALTER SYSTEM first.. This was actually linked to point 1 mentioned above, if we create empty file at initdb time, then there should not be any problem. One other option to disable as suggested by Alvaro is have another config parameter to enable/disable Alter System, if we can do that way, the solution will be much neater. >> One question here, if somebody just enables it without using ALTER >> SYSTEM, should it throw any error on not finding auto file or just >> ignore it? > > I'd prefer that it error if it can't find an included file. > >> > What about include directives? >> >> Sorry, I didn't get which parameters you are referring by include directives? > > Literally, the above 'include' in postgresql.conf. Would that only be > dealt with as a parse-time thing, or should it be more like a GUC which > could then be set through ALTER SYSTEM? I think if we choose to use include directive as a way to enable/disable this feature, it will not be good to allow change of this parameter with Alter System. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers