Re: Issue with PGC_BACKEND parameters

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Issue with PGC_BACKEND parameters
Date: 2014-01-30 19:44:47
Message-ID: 52EAABAF.3030609@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2014-01-30 19:51:57 Re: Fwd: Request for error explaination || Adding a new integer in indextupleData Structure
Previous Message Peter Geoghegan 2014-01-30 19:28:02 Re: Add min and max execute statement time in pg_stat_statement