Re: [HACKERS] Issue with PGC_BACKEND parameters
Amit Kapila amit.kapil...@gmail.com writes: [ set_guc_backend_params_v2.patch ] Committed with minor cosmetic adjustments. 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] Issue with PGC_BACKEND parameters
On 12/22/2013 11:30 PM, Amit Kapila wrote:
I had observed one problem with PGC_BACKEND parameters while testing patch
for ALTER SYSTEM command.
Problem statement: If I change PGC_BACKEND parameters directly in
postgresql.conf and then do pg_reload_conf() and reconnect, it will
still show the old value.
Detailed steps
1. Start server with default settings
2. Connect Client
3. show log_connections; -- it will show as off, this is correct.
4. Change log_connections in postgresql.conf to on
5. issue command select pg_reload_conf() in client (which is started in step-2)
6. Connect a new client
7. show log_connections; -- it will show as off, this is in-correct.
The problem is in step-7, it should show as on.
This problem occur only in Windows.
The reason for this problem is that in WINDOWS, when a new session is
started it will load the changed parameters in new backend by
global/config_exec_params file. The flow is in
SubPostmasterMain()-read_nondefault_variables()-set_config_option().
In below code in function set_config_option(), it will not allow to change
PGC_BACKEND variable and even in comments it has mentioned that only
postmaster will be allowed to change and the same will propagate to
subsequently started backends, but this is not TRUE for Windows.
switch (record-context)
{
..
..
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
* If a PGC_BACKEND parameter is changed in
the config file,
* we want to accept the new value in the
postmaster (whence
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.
This is a tad klugy, but
* necessary because we don't re-read the
config file during
* backend start.
*/
if (IsUnderPostmaster)
return -1;
}
}
I think to fix the issue we need to pass the information whether PGC_BACKEND
parameter is allowed to change in set_config_option() function.
One way is to pass a new parameter.
Yesterday, I again thought about this issue and found that we can handle it by
checking IsInitProcessingMode() which will be True only during backend startup
which is what we need here.
Please find the attached patch to fix this issue.
I think that this issue should be fixed in PostgreSQL, because
currently PGC_BACKEND
parameters doesn't work on Windows.
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5605,9 +5605,12 @@ set_config_option(const char *name, const char
*value,
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.This is a tad
klugy, but
* necessary because we don't re-read the config file
during
- * backend start.
+ * backend start. However for windows, we need to process
+ * config file during backend start for non-default
parameters,
+ * so we need to allow change of PGC_BACKEND during backend
+ * startup.
*/
-if (IsUnderPostmaster)
+if (IsUnderPostmaster !IsInitProcessingMode())
return -1;
}
else if (context != PGC_POSTMASTER context !=
PGC_BACKEND
I think this change looks OK. Does anyone else have any comments before
I test and apply it? I presume it's a bugfix that should be backpatched.
cheers
andrew
--
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] Issue with PGC_BACKEND parameters
Andrew Dunstan and...@dunslane.net writes: On 12/22/2013 11:30 PM, Amit Kapila wrote: - * backend start. + * backend start. However for windows, we need to process + * config file during backend start for non-default parameters, + * so we need to allow change of PGC_BACKEND during backend + * startup. */ -if (IsUnderPostmaster) +if (IsUnderPostmaster !IsInitProcessingMode()) return -1; } I think this change looks OK. The comment is pretty awful, since this is neither Windows-specific nor a read of the config file. Perhaps more like However, in EXEC_BACKEND builds we load nondefault settings from the CONFIG_EXEC_PARAMS file during backend start. In that situation we should accept PGC_SIGHUP settings, so as to have the same value as if we'd forked from the postmaster. Also, I think that the extra test should only be made #ifdef EXEC_BACKEND, so as to minimize the risk of breaking things. Not that this isn't pretty darn fragile anyway; I think testing IsInitProcessingMode here is a very random way to detect this case. I wonder if it'd be better to pass down an explicit flag indicating that we're doing read_nondefault_variables(). If we don't do that, maybe an Assert(IsInitProcessingMode()) in read_nondefault_variables() would be a good thing. 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] Issue with PGC_BACKEND parameters
On 01/30/2014 03:12 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 12/22/2013 11:30 PM, Amit Kapila wrote: - * backend start. + * backend start. However for windows, we need to process + * config file during backend start for non-default parameters, + * so we need to allow change of PGC_BACKEND during backend + * startup. */ -if (IsUnderPostmaster) +if (IsUnderPostmaster !IsInitProcessingMode()) return -1; } I think this change looks OK. The comment is pretty awful, since this is neither Windows-specific nor a read of the config file. Perhaps more like However, in EXEC_BACKEND builds we load nondefault settings from the CONFIG_EXEC_PARAMS file during backend start. In that situation we should accept PGC_SIGHUP settings, so as to have the same value as if we'd forked from the postmaster. Also, I think that the extra test should only be made #ifdef EXEC_BACKEND, so as to minimize the risk of breaking things. Not that this isn't pretty darn fragile anyway; I think testing IsInitProcessingMode here is a very random way to detect this case. I wonder if it'd be better to pass down an explicit flag indicating that we're doing read_nondefault_variables(). If we don't do that, maybe an Assert(IsInitProcessingMode()) in read_nondefault_variables() would be a good thing. OK, I've added your comment to the commitfest item and marked it as Waiting on Author. cheers andrew -- 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] Issue with PGC_BACKEND parameters
On Fri, Jan 31, 2014 at 2:01 AM, Andrew Dunstan and...@dunslane.net wrote: On 01/30/2014 03:12 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: On 12/22/2013 11:30 PM, Amit Kapila wrote: - * backend start. + * backend start. However for windows, we need to process + * config file during backend start for non-default parameters, + * so we need to allow change of PGC_BACKEND during backend + * startup. */ -if (IsUnderPostmaster) +if (IsUnderPostmaster !IsInitProcessingMode()) return -1; } I think this change looks OK. The comment is pretty awful, since this is neither Windows-specific nor a read of the config file. Perhaps more like However, in EXEC_BACKEND builds we load nondefault settings from the CONFIG_EXEC_PARAMS file during backend start. In that situation we should accept PGC_SIGHUP settings, so as to have the same value as if we'd forked from the postmaster. Changed as per suggestion. Also, I think that the extra test should only be made #ifdef EXEC_BACKEND, so as to minimize the risk of breaking things. Agreed and changed the patch as per suggestion. Not that this isn't pretty darn fragile anyway; I think testing IsInitProcessingMode here is a very random way to detect this case. I wonder if it'd be better to pass down an explicit flag indicating that we're doing read_nondefault_variables(). My first idea was to add a parameter, but set_config_option is getting called from multiple places and this case doesn't seem to be generic enough to add a parameter to commonly used function, so I found another way of doing it. I agree that adding a new parameter would be a better fix, but just seeing the places from where it get called, I thought of doing it other way, however if you feel strongly about it, I can change the patch to pass a new parameter to set_config_option(). If we don't do that, maybe an Assert(IsInitProcessingMode()) in read_nondefault_variables() would be a good thing. Added Assert in read_nondefault_variables(). OK, I've added your comment to the commitfest item and marked it as Waiting on Author. Thanks for Review. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com set_guc_backend_params_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] Issue with PGC_BACKEND parameters
I had observed one problem with PGC_BACKEND parameters while testing patch
for ALTER SYSTEM command.
Problem statement: If I change PGC_BACKEND parameters directly in
postgresql.conf and then do pg_reload_conf() and reconnect, it will
still show the old value.
Detailed steps
1. Start server with default settings
2. Connect Client
3. show log_connections; -- it will show as off, this is correct.
4. Change log_connections in postgresql.conf to on
5. issue command select pg_reload_conf() in client (which is started in
step-2)
6. Connect a new client
7. show log_connections; -- it will show as off, this is in-correct.
The problem is in step-7, it should show as on.
This problem occur only in Windows.
The reason for this problem is that in WINDOWS, when a new session is
started it will load the changed parameters in new backend by
global/config_exec_params file. The flow is in
SubPostmasterMain()-read_nondefault_variables()-set_config_option().
In below code in function set_config_option(), it will not allow to change
PGC_BACKEND variable and even in comments it has mentioned that only
postmaster will be allowed to change and the same will propagate to
subsequently started backends, but this is not TRUE for Windows.
switch (record-context)
{
..
..
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
* If a PGC_BACKEND parameter is changed in
the config file,
* we want to accept the new value in the
postmaster (whence
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.
This is a tad klugy, but
* necessary because we don't re-read the
config file during
* backend start.
*/
if (IsUnderPostmaster)
return -1;
}
}
I think to fix the issue we need to pass the information whether PGC_BACKEND
parameter is allowed to change in set_config_option() function.
One way is to pass a new parameter.
Yesterday, I again thought about this issue and found that we can handle it by
checking IsInitProcessingMode() which will be True only during backend startup
which is what we need here.
Please find the attached patch to fix this issue.
I think that this issue should be fixed in PostgreSQL, because
currently PGC_BACKEND
parameters doesn't work on Windows.
I will upload this patch to next CF, so that it can be tracked.
Kindly let me know your suggestions or if you have any objections?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
set_guc_backend_params.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
[HACKERS] Issue with PGC_BACKEND parameters
I had observed one problem with PGC_BACKEND parameters while testing patch
for ALTER SYSTEM command.
Problem statement: If I change PGC_BACKEND parameters directly in
postgresql.conf and then do pg_reload_conf() and reconnect, it will
still show the old value.
Detailed steps
1. Start server with default settings
2. Connect Client
3. show log_connections; -- it will show as off, this is correct.
4. Change log_connections in postgresql.conf to on
5. issue command select pg_reload_conf() in client (which is started in
step-2)
6. Connect a new client
7. show log_connections; -- it will show as off, this is in-correct.
The problem is in step-7, it should show as on.
This problem occur only in Windows.
The reason for this problem is that in WINDOWS, when a new session is
started it will load the changed parameters in new backend by
global/config_exec_params file. The flow is in
SubPostmasterMain()-read_nondefault_variables()-set_config_option().
In below code in function set_config_option(), it will not allow to change
PGC_BACKEND variable and even in comments it has mentioned that only
postmaster will be allowed to change and the same will propagate to
subsequently started backends, but this is not TRUE for Windows.
switch (record-context)
{
..
..
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
/*
* If a PGC_BACKEND parameter is changed in
the config file,
* we want to accept the new value in the
postmaster (whence
* it will propagate to subsequently-started
backends), but
* ignore it in existing backends.
This is a tad klugy, but
* necessary because we don't re-read the
config file during
* backend start.
*/
if (IsUnderPostmaster)
return -1;
}
}
I think to fix the issue we need to pass the information whether PGC_BACKEND
parameter is allowed to change in set_config_option() function.
One way is to pass a new parameter.
Kindly let me know your suggestions.
With Regards,
Amit Kapila.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
