Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-19 Thread Fujii Masao
On Wed, Jun 18, 2014 at 1:07 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 At 2014-06-18 12:55:36 +0900, masao.fu...@gmail.com wrote:

 Also I'm thinking to backpatch this to 9.4 because this is a bugfix
 rather than new feature.

 That sounds reasonable, thanks.

Committed!

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-19 Thread Amit Kapila
On Thu, Jun 19, 2014 at 5:21 PM, Fujii Masao masao.fu...@gmail.com wrote:

 On Wed, Jun 18, 2014 at 1:07 PM, Abhijit Menon-Sen a...@2ndquadrant.com
wrote:
  At 2014-06-18 12:55:36 +0900, masao.fu...@gmail.com wrote:
 
  Also I'm thinking to backpatch this to 9.4 because this is a bugfix
  rather than new feature.
 
  That sounds reasonable, thanks.

 Committed!

Thank you.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-17 Thread Fujii Masao
On Tue, Jun 17, 2014 at 2:08 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 At 2014-06-17 09:49:35 +0530, amit.kapil...@gmail.com wrote:

 By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
 something else, if earlier then I have removed it as per comment from
 Fujji-san.

 Yes, what you've done in v3 of the patch is what I meant. I've marked
 this as ready for committer now.

Barring objections, I will commit this patch.

Also I'm thinking to backpatch this to 9.4 because this is a bugfix rather
than new feature.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-17 Thread Abhijit Menon-Sen
At 2014-06-18 12:55:36 +0900, masao.fu...@gmail.com wrote:

 Also I'm thinking to backpatch this to 9.4 because this is a bugfix
 rather than new feature.

That sounds reasonable, thanks.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-16 Thread Christoph Berg
Re: Amit Kapila 2014-06-16 
CAA4eK1++wcFswhzH=92qiQFB=C0_Uy=mreadvhzxdrf3jow...@mail.gmail.com
 Developer options are mainly for debugging information or might help in one
 of the situations, so I thought somebody might not want them to be part of
 server configuration once they are set.  We already disallow parameters like
 config_file, transaction_isolation, etc. which are disallowed to be set in
 postgresql.conf.  Could you please explain me a bit in which
 situations/scenarios, do you think that allowing developer options via Alter
 System can be helpful?

Oh if these are disallowed in postgresql.conf, they should be
disallowed in auto.conf too. I meant to say that there shouldn't be
additional restrictions for auto.conf, except for cases like
data_directory which won't work at all (or are highly confusing).

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-16 Thread Fujii Masao
On Mon, Jun 16, 2014 at 12:14 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Sun, Jun 15, 2014 at 6:29 PM, Christoph Berg c...@df7cb.de wrote:

 Re: Amit Kapila 2014-06-13
 CAA4eK1KLn1SmgVtd=5emabqxrrpveedtbuu94e-repmwxwv...@mail.gmail.com
  Agreed, I had mentioned in Notes section of document.  Apart from that
  I had disallowed parameters that are excluded from postgresql.conf by
  initdb (Developer options) and they are recommended in user manual
  to be not used in production.

 Excluding developer options seems too excessive to me. ALTER SYSTEM
 should be useful for developing too.

 Developer options are mainly for debugging information or might help in one
 of the situations, so I thought somebody might not want them to be part of
 server configuration once they are set.  We already disallow parameters like
 config_file, transaction_isolation, etc. which are disallowed to be set in
 postgresql.conf.  Could you please explain me a bit in which
 situations/scenarios, do you think that allowing developer options via Alter
 System can be helpful?

I think that's helpful. I've heard that some users enable the developer option
trace_sort in postgresql.conf in order to collect the information
about sorting.
They might want to set trace_sort via ALTER SYSTEM, for example.

 This information is not stored in pg_settings.  One way is to specify
 manually all the parameters which are disallowed but it seems the query
 will become clumsy, another could be to have another column in pg_settings.
 Do you think of any other way?

I like the latter idea, i.e., exposing the flags of GUC parameters in
pg_settings,
but it seems overkill just for tab-completion purpose. I'd withdraw my comment.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-16 Thread Amit Kapila
On Mon, Jun 16, 2014 at 7:06 PM, Fujii Masao masao.fu...@gmail.com wrote:
  Developer options are mainly for debugging information or might help in
one
  of the situations, so I thought somebody might not want them to be part
of
  server configuration once they are set.  We already disallow parameters
like
  config_file, transaction_isolation, etc. which are disallowed to be set
in
  postgresql.conf.  Could you please explain me a bit in which
  situations/scenarios, do you think that allowing developer options via
Alter
  System can be helpful?

 I think that's helpful. I've heard that some users enable the developer
option
 trace_sort in postgresql.conf in order to collect the information
 about sorting.
 They might want to set trace_sort via ALTER SYSTEM, for example.

Okay, if you have usecase where people use such parameters in
postegresql.conf, then it makes sense.  I have removed that restriction
from patch.

 I suggest the following wording:

 This command may not be used to set
 xref linkend=guc-data-directory
 or any parameters that are not allowed in postgresql.conf.

I think we should not use *may* in this line, because in no
circumstance we will allow this command for the parameters
mentioned.  I have changed it as per your suggestion.

  + /*
  +  * Disallow parameter's that are excluded or disallowed in
  +  * postgresql.conf.
  +  */

 parameters, no apostrophe.
okay.
I have changed above comment as earlier comment has
information about developer option which we don't need now.

if ((record-context == PGC_INTERNAL) ||
  - (record-flags  GUC_DISALLOW_IN_FILE))
  - ereport(ERROR,
  -
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  -  errmsg(parameter \%s\ cannot be
changed,
  - name)));
  + (record-flags  GUC_DISALLOW_IN_FILE) ||
  + (record-flags  GUC_DISALLOW_IN_AUTO_FILE) ||
  + (record-flags  GUC_NOT_IN_SAMPLE))
  +  ereport(ERROR,
  +
 (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
  +   errmsg(parameter \%s\ cannot be
changed,
  +  name)));

 I looked at the settings that are marked GUC_NOT_IN_SAMPLE but neither
 PGC_INTERNAL nor GUC_DISALLOW_IN_*FILE. I don't feel strongly about it,
 but I don't see any particularly good reason to exclude them here.

By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
something else, if earlier then I have removed it as per comment from
Fujji-san.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


prohibit_data_dir_by_alter_system-v3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-16 Thread Abhijit Menon-Sen
At 2014-06-17 09:49:35 +0530, amit.kapil...@gmail.com wrote:

 By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
 something else, if earlier then I have removed it as per comment from
 Fujji-san.

Yes, what you've done in v3 of the patch is what I meant. I've marked
this as ready for committer now.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-15 Thread Christoph Berg
Re: Amit Kapila 2014-06-13 
CAA4eK1KLn1SmgVtd=5emabqxrrpveedtbuu94e-repmwxwv...@mail.gmail.com
 Agreed, I had mentioned in Notes section of document.  Apart from that
 I had disallowed parameters that are excluded from postgresql.conf by
 initdb (Developer options) and they are recommended in user manual
 to be not used in production.

Excluding developer options seems too excessive to me. ALTER SYSTEM
should be useful for developing too.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-15 Thread Amit Kapila
On Sun, Jun 15, 2014 at 6:29 PM, Christoph Berg c...@df7cb.de wrote:

 Re: Amit Kapila 2014-06-13 CAA4eK1KLn1SmgVtd=
5emabqxrrpveedtbuu94e-repmwxwv...@mail.gmail.com
  Agreed, I had mentioned in Notes section of document.  Apart from that
  I had disallowed parameters that are excluded from postgresql.conf by
  initdb (Developer options) and they are recommended in user manual
  to be not used in production.

 Excluding developer options seems too excessive to me. ALTER SYSTEM
 should be useful for developing too.

Developer options are mainly for debugging information or might help in one
of the situations, so I thought somebody might not want them to be part of
server configuration once they are set.  We already disallow parameters like
config_file, transaction_isolation, etc. which are disallowed to be set in
postgresql.conf.  Could you please explain me a bit in which
situations/scenarios, do you think that allowing developer options via Alter
System can be helpful?


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-13 Thread Amit Kapila
On Thu, Jun 12, 2014 at 7:26 PM, Fujii Masao masao.fu...@gmail.com wrote:

 On Tue, May 27, 2014 at 2:05 PM, Amit Kapila amit.kapil...@gmail.com
wrote:
  On Sun, May 11, 2014 at 11:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  I think it's clearly *necessary* to forbid setting data_directory in
  postgresql.auto.conf.  The file is defined to be found in the data
  directory, so any such setting is circular logic by definition;
  no good can come of not rejecting it.
 
  We already have a GUC flag bit about disallowing certain variables
  in the config file (though I don't remember if it's enforced or
  just advisory).  It seems to me that we'd better invent one for
  disallowing in ALTER SYSTEM, as well.
 
  I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
  disallow parameters by Alter System and disallowed data_directory to
  be set by Alter System.

 We should document what types of parameters are not allowed to be set by
 ALTER SYSTEM SET?

Agreed, I had mentioned in Notes section of document.  Apart from that
I had disallowed parameters that are excluded from postgresql.conf by
initdb (Developer options) and they are recommended in user manual
to be not used in production.

 data_directory was displayed when I typed TAB just after ALTER SYSTEM
SET.
 Probably tab-completion for ALTER SYSTEM SET needs to be changed.

This information is not stored in pg_settings.  One way is to specify
manually all the parameters which are disallowed but it seems the query
will become clumsy, another could be to have another column in pg_settings.
Do you think of any other way?


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


prohibit_data_dir_by_alter_system-v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-12 Thread Fujii Masao
On Tue, May 27, 2014 at 2:05 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Sun, May 11, 2014 at 11:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I think it's clearly *necessary* to forbid setting data_directory in
 postgresql.auto.conf.  The file is defined to be found in the data
 directory, so any such setting is circular logic by definition;
 no good can come of not rejecting it.

 We already have a GUC flag bit about disallowing certain variables
 in the config file (though I don't remember if it's enforced or
 just advisory).  It seems to me that we'd better invent one for
 disallowing in ALTER SYSTEM, as well.

 I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
 disallow parameters by Alter System and disallowed data_directory to
 be set by Alter System.

We should document what types of parameters are not allowed to be set by
ALTER SYSTEM SET?

data_directory was displayed when I typed TAB just after ALTER SYSTEM SET.
Probably tab-completion for ALTER SYSTEM SET needs to be changed.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-06-11 Thread Amit Kapila
On Tue, May 27, 2014 at 10:35 AM, Amit Kapila amit.kapil...@gmail.com
wrote:
 On Sun, May 11, 2014 at 11:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  I think it's clearly *necessary* to forbid setting data_directory in
  postgresql.auto.conf.  The file is defined to be found in the data
  directory, so any such setting is circular logic by definition;
  no good can come of not rejecting it.
 
  We already have a GUC flag bit about disallowing certain variables
  in the config file (though I don't remember if it's enforced or
  just advisory).  It seems to me that we'd better invent one for
  disallowing in ALTER SYSTEM, as well.

 I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
 disallow parameters by Alter System and disallowed data_directory to
 be set by Alter System.

I have added this patch in next CF to avoid getting it lost.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-26 Thread Amit Kapila
On Sun, May 11, 2014 at 11:23 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I think it's clearly *necessary* to forbid setting data_directory in
 postgresql.auto.conf.  The file is defined to be found in the data
 directory, so any such setting is circular logic by definition;
 no good can come of not rejecting it.

 We already have a GUC flag bit about disallowing certain variables
 in the config file (though I don't remember if it's enforced or
 just advisory).  It seems to me that we'd better invent one for
 disallowing in ALTER SYSTEM, as well.

I introduced a new flag bit (GUC_DISALLOW_IN_AUTO_FILE) to
disallow parameters by Alter System and disallowed data_directory to
be set by Alter System.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


prohibit_data_dir_by_alter_system.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-11 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 In above scenario, I think you are expecting it should use
 /data2/postgresql.auto.conf and that is what you have mentioned
 up-thread.  The way to handle it by server is just to forbid setting
 this parameter
 by Alter System or the user himself should not perform such an action.
 Here if we want user to be careful of performing such an action, then may
 be it's better to have such an indication in ALTER SYSTEM documentation.

I think it's clearly *necessary* to forbid setting data_directory in
postgresql.auto.conf.  The file is defined to be found in the data
directory, so any such setting is circular logic by definition;
no good can come of not rejecting it.

We already have a GUC flag bit about disallowing certain variables
in the config file (though I don't remember if it's enforced or
just advisory).  It seems to me that we'd better invent one for
disallowing in ALTER SYSTEM, as well.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-11 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 This problem occurs because we don't have the value of data_directory
 set in postgresql.conf by the time we want to parse .auto.conf file
 during server start.  The value of data_directory is only available after
 processing of config files.  To fix it, we need to store the value of
 data_directory during parse of postgresql.conf file so that we can use it
 till data_directory is actually set.  Attached patch fixes the problem.

Aside from being about as ugly as it could possibly be, this is wrong,
because it only covers one of the possible sources of a data_directory
that's different from the config file location.  In particular, it's
possible to set --config_file=something on the postmaster command line
and also set a data directory different than that via -D or a PGDATA
environment variable, rather than an entry in postgresql.conf (see the
initial logic in SelectConfigFiles).

While it's possible that you could deal with that with some even uglier
variant on this patch (perhaps involving making SelectConfigFiles export
its internal value of configdir), I think that having ProcessConfigFile
understand exactly what SelectConfigFiles is going to do to select the
final value of data_directory would be a clear modularity violation,
and very fragile as well.

I think what probably has to happen is that ProcessConfigFile shouldn't
be internally responsible for reading the auto file at all, but that we
do that via two separate calls to ProcessConfigFile, one for the main
file and then one for the auto file; and during initial startup,
SelectConfigFiles doesn't make the call for the auto file until after
it's established the final value of data_directory.  (And by the by,
I think that DataDir not data_directory is what to be using to compute
the auto file's path.)

It's distressing that this wasn't noticed till now; it shows that nobody
tested ALTER SYSTEM with a config file outside the data directory, which
seems like a rather fundamental omission for any work with GUC files.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-11 Thread Tom Lane
I wrote:
 I think what probably has to happen is that ProcessConfigFile shouldn't
 be internally responsible for reading the auto file at all, but that we
 do that via two separate calls to ProcessConfigFile, one for the main
 file and then one for the auto file; and during initial startup,
 SelectConfigFiles doesn't make the call for the auto file until after
 it's established the final value of data_directory.

Since this bug would block testing of ALTER SYSTEM by a nontrivial
population of users, I felt it was important to get it fixed before beta,
so I went to try and fix it as above.  It turns out that reading the two
config files separately doesn't work because ProcessConfigFile will think
all the settings got removed from the file.  What we can do for the
moment, though, is to just run ProcessConfigFile twice during startup,
skipping the auto file the first time.  It might be worth refactoring
ProcessConfigFile to avoid the overhead of reading postgresql.conf twice,
but it would be a lot of work and I think the gain would be marginal; in
any case there's not time to do that today.  But I've got the main problem
fixed in time for beta.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-11 Thread Robert Haas
On Sun, May 11, 2014 at 3:20 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I wrote:
 I think what probably has to happen is that ProcessConfigFile shouldn't
 be internally responsible for reading the auto file at all, but that we
 do that via two separate calls to ProcessConfigFile, one for the main
 file and then one for the auto file; and during initial startup,
 SelectConfigFiles doesn't make the call for the auto file until after
 it's established the final value of data_directory.

 Since this bug would block testing of ALTER SYSTEM by a nontrivial
 population of users, I felt it was important to get it fixed before beta,
 so I went to try and fix it as above.  It turns out that reading the two
 config files separately doesn't work because ProcessConfigFile will think
 all the settings got removed from the file.  What we can do for the
 moment, though, is to just run ProcessConfigFile twice during startup,
 skipping the auto file the first time.  It might be worth refactoring
 ProcessConfigFile to avoid the overhead of reading postgresql.conf twice,
 but it would be a lot of work and I think the gain would be marginal; in
 any case there's not time to do that today.  But I've got the main problem
 fixed in time for beta.

Thanks for dealing with this.  I was skeptical of the proposed patch,
but didn't have time to think hard enough about it to say something
intelligent.  I'm glad you did.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-11 Thread Amit Kapila
On Mon, May 12, 2014 at 12:50 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Since this bug would block testing of ALTER SYSTEM by a nontrivial
 population of users, I felt it was important to get it fixed before beta,
 so I went to try and fix it as above.  It turns out that reading the two
 config files separately doesn't work because ProcessConfigFile will think
 all the settings got removed from the file.  What we can do for the
 moment, though, is to just run ProcessConfigFile twice during startup,
 skipping the auto file the first time.  It might be worth refactoring
 ProcessConfigFile to avoid the overhead of reading postgresql.conf twice,
 but it would be a lot of work and I think the gain would be marginal; in
 any case there's not time to do that today.  But I've got the main problem
 fixed in time for beta.

Thanks for fixing it.
I will provide a fix to forbid data_directory via Alter System as suggested
by you upthread.

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: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-10 Thread Fujii Masao
On Sat, May 10, 2014 at 12:30 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Fri, May 9, 2014 at 7:01 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Fri, May 9, 2014 at 1:06 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 There is no harm in forbidding data_directory, but similarly we can
 imagine that people can set some very large values for some config
 variables due to which later it can have symptoms similar to this
 issue.

 Yes, that can prevent the server from restarting at all. In this case,
 to restart the server, we need to edit postgresql.auto.conf manually
 and remove the problematic settings though the header of
 postgresql.auto.conf warns Do not edit this file manually!.

 So lets not try to forbid data_directory in postgresql.auto.conf and just
 fix the case reported in this mail for postgresql.conf for which the
 patch is attached upthread.

ISTM that data_directory is in different situation from the other. That is,
setting data_directory in postgresql.auto.conf is problematic whether its
setting value is valid or invalid. Imagine the case where data_directory
is set to '/data1' and '/data2' in config_directory/postgresql.conf and
/data1/postgresql.auto.conf, respectively. In this case, firstly the server
doesn't read /data2/postgresql.auto.conf when it starts up, even if your
patch has been applied. Secondly, the server doesn't read
/data1/postgresql.auto.conf when it receives SIGHUP, and this causes
the following error.

LOG:  parameter data_directory cannot be changed without restarting the server

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-10 Thread Amit Kapila
On Sun, May 11, 2014 at 4:38 AM, Fujii Masao masao.fu...@gmail.com wrote:
 ISTM that data_directory is in different situation from the other. That is,
 setting data_directory in postgresql.auto.conf is problematic whether its
 setting value is valid or invalid. Imagine the case where data_directory
 is set to '/data1' and '/data2' in config_directory/postgresql.conf and
 /data1/postgresql.auto.conf, respectively. In this case, firstly the server
 doesn't read /data2/postgresql.auto.conf when it starts up, even if your
 patch has been applied.

I think after my patch, first server will read data1/postgresql.auto.conf
on start as that is what is set in postgresql.conf.

 Secondly, the server doesn't read
 /data1/postgresql.auto.conf when it receives SIGHUP, and this causes
 the following error.

 LOG:  parameter data_directory cannot be changed without restarting the 
 server

This happens because data_directory is PGC_POSTMASTER parameter
and none of such parameters can be changed with SIGHUP.

In above scenario, I think you are expecting it should use
/data2/postgresql.auto.conf and that is what you have mentioned
up-thread.  The way to handle it by server is just to forbid setting
this parameter
by Alter System or the user himself should not perform such an action.
Here if we want user to be careful of performing such an action, then may
be it's better to have such an indication in ALTER SYSTEM documentation.


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: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-09 Thread Fujii Masao
On Fri, May 9, 2014 at 1:06 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, May 8, 2014 at 9:47 PM, Christoph Berg c...@df7cb.de wrote:
 Re: Andres Freund 2014-05-08 20140508145901.gb1...@awork2.anarazel.de
  Maybe this is nitpicking, but what happens when postgresql.auto.conf also
  includes the setting of data_directory? This is possible because we can
  set data_directory via ALTER SYSTEM now. Should we just ignore such
  problematic setting in postgresql.auto.conf with warning message?

 I think that's a case of Doctor, it hurts when I do this. Doctor: don't
 do that then.

 I'd opt to forbid setting data_directory at ALTER SYSTEM time. For the
 other options, I agree with Andres that you should get to keep all
 parts if you manage to break it.

 There is no harm in forbidding data_directory, but similarly we can
 imagine that people can set some very large values for some config
 variables due to which later it can have symptoms similar to this
 issue.

Yes, that can prevent the server from restarting at all. In this case,
to restart the server, we need to edit postgresql.auto.conf manually
and remove the problematic settings though the header of
postgresql.auto.conf warns Do not edit this file manually!.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-09 Thread Amit Kapila
On Fri, May 9, 2014 at 7:01 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Fri, May 9, 2014 at 1:06 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 There is no harm in forbidding data_directory, but similarly we can
 imagine that people can set some very large values for some config
 variables due to which later it can have symptoms similar to this
 issue.

 Yes, that can prevent the server from restarting at all. In this case,
 to restart the server, we need to edit postgresql.auto.conf manually
 and remove the problematic settings though the header of
 postgresql.auto.conf warns Do not edit this file manually!.

So lets not try to forbid data_directory in postgresql.auto.conf and just
fix the case reported in this mail for postgresql.conf for which the
patch is attached upthread.

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: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-08 Thread Fujii Masao
On Wed, May 7, 2014 at 4:57 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Tue, May 6, 2014 at 7:04 PM, Christoph Berg c...@df7cb.de wrote:
 Hi,

 if you split configuration and data by setting data_directory,
 postgresql.auto.conf is writen to the data directory
 (/var/lib/postgresql/9.4/main in Debian), but tried to be read from
 the etc directory (/etc/postgresql/9.4/main).

 One place to fix it would be in ProcessConfigFile in
 src/backend/utils/misc/guc-file.l:162 by always setting
 CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
 that breaks later in AbsoluteConfigLocation() when data_directory is
 NULL. (As the comment in ProcessConfigFile says.)

 This problem occurs because we don't have the value of data_directory
 set in postgresql.conf by the time we want to parse .auto.conf file
 during server start.  The value of data_directory is only available after
 processing of config files.  To fix it, we need to store the value of
 data_directory during parse of postgresql.conf file so that we can use it
 till data_directory is actually set.  Attached patch fixes the problem.
 Could you please once confirm if it fixes the problem in your
 env./scenario.

Maybe this is nitpicking, but what happens when postgresql.auto.conf also
includes the setting of data_directory? This is possible because we can
set data_directory via ALTER SYSTEM now. Should we just ignore such
problematic setting in postgresql.auto.conf with warning message?

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-08 Thread Amit Kapila
On Thu, May 8, 2014 at 6:51 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Wed, May 7, 2014 at 4:57 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 This problem occurs because we don't have the value of data_directory
 set in postgresql.conf by the time we want to parse .auto.conf file
 during server start.  The value of data_directory is only available after
 processing of config files.  To fix it, we need to store the value of
 data_directory during parse of postgresql.conf file so that we can use it
 till data_directory is actually set.  Attached patch fixes the problem.
 Could you please once confirm if it fixes the problem in your
 env./scenario.

 Maybe this is nitpicking, but what happens when postgresql.auto.conf also
 includes the setting of data_directory? This is possible because we can
 set data_directory via ALTER SYSTEM now.

   Yes, that will also be issue similar to above.

 Should we just ignore such
 problematic setting in postgresql.auto.conf with warning message?

Another way could be that we detect the same during server start
(while parsing postgresql.auto.conf) and then allow for reparse of
auto conf file, but I think that will be bit more complicated.  So lets
try to solve it in a way suggested by you.  If there is no objection by
anyone else then I will update the patch.

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: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-08 Thread Stephen Frost
* Amit Kapila (amit.kapil...@gmail.com) wrote:
 On Thu, May 8, 2014 at 6:51 PM, Fujii Masao masao.fu...@gmail.com wrote:
  Should we just ignore such
  problematic setting in postgresql.auto.conf with warning message?
 
 Another way could be that we detect the same during server start
 (while parsing postgresql.auto.conf) and then allow for reparse of
 auto conf file, but I think that will be bit more complicated.  So lets
 try to solve it in a way suggested by you.  If there is no objection by
 anyone else then I will update the patch.

Pretty sure this is more-or-less exactly what I suggested quite a while
back during the massive ALTER SYSTEM SET thread..  There are certain
options which we shouldn't be allowing in .auto.conf.  I doubt this is
going to be the only one...

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-08 Thread Andres Freund
On 2014-05-08 22:21:43 +0900, Fujii Masao wrote:
 On Wed, May 7, 2014 at 4:57 PM, Amit Kapila amit.kapil...@gmail.com wrote:
  On Tue, May 6, 2014 at 7:04 PM, Christoph Berg c...@df7cb.de wrote:
  Hi,
 
  if you split configuration and data by setting data_directory,
  postgresql.auto.conf is writen to the data directory
  (/var/lib/postgresql/9.4/main in Debian), but tried to be read from
  the etc directory (/etc/postgresql/9.4/main).
 
  One place to fix it would be in ProcessConfigFile in
  src/backend/utils/misc/guc-file.l:162 by always setting
  CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
  that breaks later in AbsoluteConfigLocation() when data_directory is
  NULL. (As the comment in ProcessConfigFile says.)
 
  This problem occurs because we don't have the value of data_directory
  set in postgresql.conf by the time we want to parse .auto.conf file
  during server start.  The value of data_directory is only available after
  processing of config files.  To fix it, we need to store the value of
  data_directory during parse of postgresql.conf file so that we can use it
  till data_directory is actually set.  Attached patch fixes the problem.
  Could you please once confirm if it fixes the problem in your
  env./scenario.
 
 Maybe this is nitpicking, but what happens when postgresql.auto.conf also
 includes the setting of data_directory? This is possible because we can
 set data_directory via ALTER SYSTEM now. Should we just ignore such
 problematic setting in postgresql.auto.conf with warning message?

I think that's a case of Doctor, it hurts when I do this. Doctor: don't
do that then.

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: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-08 Thread Christoph Berg
Re: Andres Freund 2014-05-08 20140508145901.gb1...@awork2.anarazel.de
  Maybe this is nitpicking, but what happens when postgresql.auto.conf also
  includes the setting of data_directory? This is possible because we can
  set data_directory via ALTER SYSTEM now. Should we just ignore such
  problematic setting in postgresql.auto.conf with warning message?
 
 I think that's a case of Doctor, it hurts when I do this. Doctor: don't
 do that then.

I'd opt to forbid setting data_directory at ALTER SYSTEM time. For the
other options, I agree with Andres that you should get to keep all
parts if you manage to break it.

Fortunately, ALTER SYSTEM already refuses to change config_file :)

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-08 Thread Amit Kapila
On Thu, May 8, 2014 at 9:47 PM, Christoph Berg c...@df7cb.de wrote:
 Re: Andres Freund 2014-05-08 20140508145901.gb1...@awork2.anarazel.de
  Maybe this is nitpicking, but what happens when postgresql.auto.conf also
  includes the setting of data_directory? This is possible because we can
  set data_directory via ALTER SYSTEM now. Should we just ignore such
  problematic setting in postgresql.auto.conf with warning message?

 I think that's a case of Doctor, it hurts when I do this. Doctor: don't
 do that then.

 I'd opt to forbid setting data_directory at ALTER SYSTEM time. For the
 other options, I agree with Andres that you should get to keep all
 parts if you manage to break it.

There is no harm in forbidding data_directory, but similarly we can
imagine that people can set some very large values for some config
variables due to which later it can have symptoms similar to this
issue.

 Fortunately, ALTER SYSTEM already refuses to change config_file :)

That is because config_file is not a file parameter (not present in
postgresql.conf).  We disallow non-file and internal (like wal_segment_size)
parameters in 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


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-07 Thread Amit Kapila
On Tue, May 6, 2014 at 7:04 PM, Christoph Berg c...@df7cb.de wrote:
 Hi,

 if you split configuration and data by setting data_directory,
 postgresql.auto.conf is writen to the data directory
 (/var/lib/postgresql/9.4/main in Debian), but tried to be read from
 the etc directory (/etc/postgresql/9.4/main).

 One place to fix it would be in ProcessConfigFile in
 src/backend/utils/misc/guc-file.l:162 by always setting
 CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
 that breaks later in AbsoluteConfigLocation() when data_directory is
 NULL. (As the comment in ProcessConfigFile says.)

This problem occurs because we don't have the value of data_directory
set in postgresql.conf by the time we want to parse .auto.conf file
during server start.  The value of data_directory is only available after
processing of config files.  To fix it, we need to store the value of
data_directory during parse of postgresql.conf file so that we can use it
till data_directory is actually set.  Attached patch fixes the problem.
Could you please once confirm if it fixes the problem in your
env./scenario.

Another way to fix could be that during parse, we directly set the config
value of data_directory, but I didn't prefer that way because the parse
routine is called from multiple paths which later on process the values
separately.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


fix_processing_auto_conf-v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-07 Thread Christoph Berg
Re: Amit Kapila 2014-05-07 
caa4ek1ktjkpvmnkos2gfnnh2zsko4ggdspswshjbq1cpu9e...@mail.gmail.com
 This problem occurs because we don't have the value of data_directory
 set in postgresql.conf by the time we want to parse .auto.conf file
 during server start.  The value of data_directory is only available after
 processing of config files.  To fix it, we need to store the value of
 data_directory during parse of postgresql.conf file so that we can use it
 till data_directory is actually set.  Attached patch fixes the problem.
 Could you please once confirm if it fixes the problem in your
 env./scenario.

Hi Amit,

the patch fixes the problem. Thanks for looking into this.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] postgresql.auto.conf read from wrong directory

2014-05-06 Thread Amit Kapila
On Tue, May 6, 2014 at 7:04 PM, Christoph Berg c...@df7cb.de wrote:
 Hi,

 if you split configuration and data by setting data_directory,
 postgresql.auto.conf is writen to the data directory
 (/var/lib/postgresql/9.4/main in Debian), but tried to be read from
 the etc directory (/etc/postgresql/9.4/main).

 One place to fix it would be in ProcessConfigFile in
 src/backend/utils/misc/guc-file.l:162 by always setting
 CallingFileName = NULL in src/backend/utils/misc/guc-file.l:162, but
 that breaks later in AbsoluteConfigLocation() when data_directory is
 NULL. (As the comment in ProcessConfigFile says.)

 I'm not sure where to break that dependency loop - the fix itself
 seems easy, just don't tell to look for postgresql.auto.conf next to
 ConfigFileName, but in the data_directory.

Thanks for the report, will look into it.

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