Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e05b866

Browse files
committed
Split PGC_S_DEFAULT into two values, for true boot_val vs computed default.
Failure to distinguish these cases is the real cause behind the recent reports of Windows builds crashing on 'infinity'::timestamp, which was directly due to failure to establish a value of timezone_abbreviations in postmaster child processes. The postmaster had the desired value, but write_one_nondefault_variable() didn't transmit it to backends. To fix that, invent a new value PGC_S_DYNAMIC_DEFAULT, and be sure to use that or PGC_S_ENV_VAR (as appropriate) for "default" settings that are computed during initialization. (We need both because there's at least one variable that could receive a value from either source.) This commit also fixes ProcessConfigFile's failure to restore the correct default value for certain GUC variables if they are set in postgresql.conf and then removed/commented out of the file. We have to recompute and reinstall the value for any GUC variable that could have received a value from PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR sources, and there were a number of oversights. (That whole thing is a crock that needs to be redesigned, but not today.) However, I intentionally didn't make it work "exactly right" for the cases of timezone and log_timezone. The exactly right behavior would involve running select_default_timezone, which we'd have to do independently in each postgres process, causing the whole database to become entirely unresponsive for as much as several seconds. That didn't seem like a good idea, especially since the variable's removal from postgresql.conf might be just an accidental edit. Instead the behavior is to adopt the previously active setting as if it were default. Note that this patch creates an ABI break for extensions that use any of the PGC_S_XXX constants; they'll need to be recompiled.
1 parent 6fc6686 commit e05b866

File tree

5 files changed

+76
-40
lines changed

5 files changed

+76
-40
lines changed

src/backend/utils/init/postinit.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ CheckMyDatabase(const char *name, bool am_superuser)
324324
PGC_INTERNAL, PGC_S_OVERRIDE);
325325
/* If we have no other source of client_encoding, use server encoding */
326326
SetConfigOption("client_encoding", GetDatabaseEncodingName(),
327-
PGC_BACKEND, PGC_S_DEFAULT);
327+
PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT);
328328

329329
/* assign locale variables */
330330
collate = NameStr(dbform->datcollate);

src/backend/utils/misc/guc-file.l

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <ctype.h>
1515
#include <unistd.h>
1616

17+
#include "mb/pg_wchar.h"
1718
#include "miscadmin.h"
1819
#include "storage/fd.h"
1920
#include "utils/guc.h"
@@ -109,7 +110,6 @@ ProcessConfigFile(GucContext context)
109110
*tail;
110111
char *cvc = NULL;
111112
struct config_string *cvc_struct;
112-
const char *envvar;
113113
int i;
114114
115115
Assert(context == PGC_POSTMASTER || context == PGC_SIGHUP);
@@ -265,7 +265,7 @@ ProcessConfigFile(GucContext context)
265265
stack->source = PGC_S_DEFAULT;
266266
}
267267

268-
/* Now we can re-apply the wired-in default */
268+
/* Now we can re-apply the wired-in default (i.e., the boot_val) */
269269
set_config_option(gconf->name, NULL, context, PGC_S_DEFAULT,
270270
GUC_ACTION_SET, true);
271271
if (context == PGC_SIGHUP)
@@ -275,25 +275,28 @@ ProcessConfigFile(GucContext context)
275275
}
276276

277277
/*
278-
* Restore any variables determined by environment variables. This
279-
* is a no-op except in the case where one of these had been in the
280-
* config file and is now removed. PGC_S_ENV_VAR will override the
281-
* wired-in default we just applied, but cannot override any other source.
278+
* Restore any variables determined by environment variables or
279+
* dynamically-computed defaults. This is a no-op except in the case
280+
* where one of these had been in the config file and is now removed.
282281
*
283-
* Keep this list in sync with InitializeGUCOptions()!
284-
* PGPORT can be ignored, because it cannot be changed without restart.
285-
* We assume rlimit hasn't changed, either.
282+
* In particular, we *must not* do this during the postmaster's
283+
* initial loading of the file, since the timezone functions in
284+
* particular should be run only after initialization is complete.
285+
*
286+
* XXX this is an unmaintainable crock, because we have to know how
287+
* to set (or at least what to call to set) every variable that could
288+
* potentially have PGC_S_DYNAMIC_DEFAULT or PGC_S_ENV_VAR source.
289+
* However, there's no time to redesign it for 9.1.
286290
*/
287-
envvar = getenv("PGDATESTYLE");
288-
if (envvar != NULL)
289-
set_config_option("datestyle", envvar, PGC_POSTMASTER,
290-
PGC_S_ENV_VAR, GUC_ACTION_SET, true);
291-
292-
envvar = getenv("PGCLIENTENCODING");
293-
if (envvar != NULL)
294-
set_config_option("client_encoding", envvar, PGC_POSTMASTER,
295-
PGC_S_ENV_VAR, GUC_ACTION_SET, true);
296-
291+
if (context == PGC_SIGHUP)
292+
{
293+
InitializeGUCOptionsFromEnvironment();
294+
pg_timezone_initialize();
295+
pg_timezone_abbrev_initialize();
296+
/* this selects SQL_ASCII in processes not connected to a database */
297+
SetConfigOption("client_encoding", GetDatabaseEncodingName(),
298+
PGC_BACKEND, PGC_S_DYNAMIC_DEFAULT);
299+
}
297300

298301
/* If we got here all the options checked out okay, so apply them. */
299302
for (item = head; item; item = item->next)

src/backend/utils/misc/guc.c

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,7 @@ const char *const GucContext_Names[] =
502502
const char *const GucSource_Names[] =
503503
{
504504
/* PGC_S_DEFAULT */ "default",
505+
/* PGC_S_DYNAMIC_DEFAULT */ "default",
505506
/* PGC_S_ENV_VAR */ "environment variable",
506507
/* PGC_S_FILE */ "configuration file",
507508
/* PGC_S_ARGV */ "command line",
@@ -3269,6 +3270,7 @@ static int GUCNestLevel = 0; /* 1 when in main transaction */
32693270

32703271
static int guc_var_compare(const void *a, const void *b);
32713272
static int guc_name_compare(const char *namea, const char *nameb);
3273+
static void InitializeGUCOptionsFromEnvironment(void);
32723274
static void InitializeOneGUCOption(struct config_generic * gconf);
32733275
static void push_old_value(struct config_generic * gconf, GucAction action);
32743276
static void ReportGUCOption(struct config_generic * record);
@@ -3812,8 +3814,6 @@ void
38123814
InitializeGUCOptions(void)
38133815
{
38143816
int i;
3815-
char *env;
3816-
long stack_rlimit;
38173817

38183818
/*
38193819
* Before log_line_prefix could possibly receive a nonempty setting, make
@@ -3852,9 +3852,25 @@ InitializeGUCOptions(void)
38523852

38533853
/*
38543854
* For historical reasons, some GUC parameters can receive defaults from
3855-
* environment variables. Process those settings. NB: if you add or
3856-
* remove anything here, see also ProcessConfigFile().
3855+
* environment variables. Process those settings.
38573856
*/
3857+
InitializeGUCOptionsFromEnvironment();
3858+
}
3859+
3860+
/*
3861+
* Assign any GUC values that can come from the server's environment.
3862+
*
3863+
* This is called from InitializeGUCOptions, and also from ProcessConfigFile
3864+
* to deal with the possibility that a setting has been removed from
3865+
* postgresql.conf and should now get a value from the environment.
3866+
* (The latter is a kludge that should probably go away someday; if so,
3867+
* fold this back into InitializeGUCOptions.)
3868+
*/
3869+
static void
3870+
InitializeGUCOptionsFromEnvironment(void)
3871+
{
3872+
char *env;
3873+
long stack_rlimit;
38583874

38593875
env = getenv("PGPORT");
38603876
if (env != NULL)
@@ -6334,6 +6350,7 @@ define_custom_variable(struct config_generic * variable)
63346350
switch (pHolder->gen.source)
63356351
{
63366352
case PGC_S_DEFAULT:
6353+
case PGC_S_DYNAMIC_DEFAULT:
63376354
case PGC_S_ENV_VAR:
63386355
case PGC_S_FILE:
63396356
case PGC_S_ARGV:
@@ -8420,15 +8437,13 @@ assign_timezone_abbreviations(const char *newval, void *extra)
84208437
*
84218438
* This is called after initial loading of postgresql.conf. If no
84228439
* timezone_abbreviations setting was found therein, select default.
8440+
* If a non-default value is already installed, nothing will happen.
84238441
*/
84248442
void
84258443
pg_timezone_abbrev_initialize(void)
84268444
{
8427-
if (timezone_abbreviations_string == NULL)
8428-
{
8429-
SetConfigOption("timezone_abbreviations", "Default",
8430-
PGC_POSTMASTER, PGC_S_DEFAULT);
8431-
}
8445+
SetConfigOption("timezone_abbreviations", "Default",
8446+
PGC_POSTMASTER, PGC_S_DYNAMIC_DEFAULT);
84328447
}
84338448

84348449
static const char *

src/include/utils/guc.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ typedef enum
8282
*/
8383
typedef enum
8484
{
85-
PGC_S_DEFAULT, /* wired-in default */
85+
PGC_S_DEFAULT, /* hard-wired default ("boot_val") */
86+
PGC_S_DYNAMIC_DEFAULT, /* default computed during initialization */
8687
PGC_S_ENV_VAR, /* postmaster environment variable */
8788
PGC_S_FILE, /* postgresql.conf */
8889
PGC_S_ARGV, /* postmaster command line */

src/timezone/pgtz.c

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,6 +1438,10 @@ pg_timezone_pre_initialize(void)
14381438
* This is called after initial loading of postgresql.conf. If no TimeZone
14391439
* setting was found therein, we try to derive one from the environment.
14401440
* Likewise for log_timezone.
1441+
*
1442+
* Note: this is also called from ProcessConfigFile, to re-establish valid
1443+
* GUC settings if the GUCs have been reset to default following their
1444+
* removal from postgresql.conf.
14411445
*/
14421446
void
14431447
pg_timezone_initialize(void)
@@ -1463,21 +1467,34 @@ pg_timezone_initialize(void)
14631467
log_timezone = def_tz;
14641468
}
14651469

1466-
/* Now, set the timezone GUC if it's not already set */
1467-
if (GetConfigOption("timezone", false) == NULL)
1468-
{
1469-
/* Tell GUC about the value. Will redundantly call pg_tzset() */
1470+
/*
1471+
* Now, set the timezone and log_timezone GUCs if they're still default.
1472+
* (This will redundantly call pg_tzset().)
1473+
*
1474+
* We choose to label these values PGC_S_ENV_VAR, rather than
1475+
* PGC_S_DYNAMIC_DEFAULT which would be functionally equivalent, because
1476+
* they came either from getenv("TZ") or from libc behavior that's
1477+
* determined by process environment of some kind.
1478+
*
1479+
* Note: in the case where a setting has just been removed from
1480+
* postgresql.conf, this code will not do what you might expect, namely
1481+
* call select_default_timezone() and install that value as the setting.
1482+
* Rather, the previously active setting --- typically the one from
1483+
* postgresql.conf --- will be reinstalled, relabeled as PGC_S_ENV_VAR.
1484+
* If we did try to install the "correct" default value, the effect would
1485+
* be that each postmaster child would independently run an extremely
1486+
* expensive search of the timezone database, bringing the database to its
1487+
* knees for possibly multiple seconds. This is so unpleasant, and could
1488+
* so easily be triggered quite unintentionally, that it seems better to
1489+
* violate the principle of least astonishment.
1490+
*/
1491+
if (GetConfigOptionResetString("timezone") == NULL)
14701492
SetConfigOption("timezone", pg_get_timezone_name(session_timezone),
14711493
PGC_POSTMASTER, PGC_S_ENV_VAR);
1472-
}
14731494

1474-
/* Likewise for log timezone */
1475-
if (GetConfigOption("log_timezone", false) == NULL)
1476-
{
1477-
/* Tell GUC about the value. Will redundantly call pg_tzset() */
1495+
if (GetConfigOptionResetString("log_timezone") == NULL)
14781496
SetConfigOption("log_timezone", pg_get_timezone_name(log_timezone),
14791497
PGC_POSTMASTER, PGC_S_ENV_VAR);
1480-
}
14811498
}
14821499

14831500

0 commit comments

Comments
 (0)