rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve tzparse's handling of TZDEFRULES ("posixrules") zone data. 

In the IANA timezone code, tzparse() always tries to load the zone
file named by TZDEFRULES ("posixrules").  Previously, we'd hacked
that logic to skip the load in the "lastditch" code path, which we use
only to initialize the default "GMT" zone during GUC initialization.
That's critical for a couple of reasons: since we do not support leap
seconds, we *must not* allow "GMT" to have leap seconds, and since this
case runs before the GUC subsystem is fully alive, we'd really rather
not take the risk of pg_open_tzfile throwing any errors.

However, that still left the code reading TZDEFRULES on every other
call, something we'd noticed to the extent of having added code to cache
the result so it was only done once per process not a lot of times.
Andres Freund complained about the static data space used up for the
cache; but as long as the logic was like this, there was no point in
trying to get rid of that space.

We can improve matters by looking a bit more closely at what the IANA
code actually needs the TZDEFRULES data for.  One thing it does is
that if "posixrules" is a leap-second-aware zone, the leap-second
behavior will be absorbed into every POSIX-style zone specification.
However, that's a behavior we'd really prefer to do without, since
for our purposes the end effect is to render every POSIX-style zone
name unsupported.  Otherwise, the TZDEFRULES data is used only if
the POSIX zone name specifies DST but doesn't include a transition
date rule (e.g., "EST5EDT" rather than "EST5EDT,M3.2.0,M11.1.0").
That is a minority case for our purposes --- in particular, it
never happens when tzload() invokes tzparse() to interpret a
transition date rule string found in a tzdata zone file.

Hence, if we legislate that we're going to ignore leap-second data
from "posixrules", we can postpone the TZDEFRULES load into the path
where we actually need to substitute for a missing date rule string.
That means it will never happen at all in common scenarios, making it
reasonable to dynamically allocate the cache space when it does happen.
Even when the data is already loaded, this saves some cycles in the
common code path since we avoid a memcpy of 23KB or so.  And, IMO at
least, this is a less ugly hack on the IANA logic than what we had
before, since it's not messing with the lastditch-vs-regular code paths.

Back-patch to all supported branches, not so much because this is a
critical change as that I want to keep all our copies of the IANA
timezone code in sync.

Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
This commit is contained in:
Tom Lane 2018-10-17 12:26:48 -04:00
parent 19ac2cb2ae
commit c776cd4724
2 changed files with 54 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/timezone/README
View File

@ -84,8 +84,10 @@ other exposed names.
slightly modified the API of the former, in part because it now relies
on our own pg_open_tzfile() rather than opening files for itself.
* tzparse() is adjusted to cache the result of loading the TZDEFRULES
zone, so that that's not repeated more than once per process.
* tzparse() is adjusted to avoid loading the TZDEFRULES zone unless
really necessary, and to ignore any leap-second data it may supply.
We also cache the result of loading the TZDEFRULES zone, so that
that's not repeated more than once per process.
* There's a fair amount of code we don't need and have removed,
including all the nonstandard optional APIs. We have also added

84
src/timezone/localtime.c
View File

@ -54,7 +54,7 @@ static const char gmt[] = "GMT";
* PG: We cache the result of trying to load the TZDEFRULES zone here.
* tzdefrules_loaded is 0 if not tried yet, +1 if good, -1 if failed.
*/
static struct state tzdefrules_s;
static struct state *tzdefrules_s = NULL;
static int tzdefrules_loaded = 0;
/*
@ -908,20 +908,10 @@ tzparse(const char *name, struct state *sp, bool lastditch)
stdname = name;
if (lastditch)
{
/*
* This is intentionally somewhat different from the IANA code. We do
* not want to invoke tzload() in the lastditch case: we can't assume
* pg_open_tzfile() is sane yet, and we don't care about leap seconds
* anyway.
*/
/* Unlike IANA, don't assume name is exactly "GMT" */
stdlen = strlen(name); /* length of standard zone name */
name += stdlen;
if (stdlen >= sizeof sp->chars)
stdlen = (sizeof sp->chars) - 1;
charcnt = stdlen + 1;
stdoffset = 0;
sp->goback = sp->goahead = false; /* simulate failed tzload() */
load_ok = false;
}
else
{
@ -945,27 +935,23 @@ tzparse(const char *name, struct state *sp, bool lastditch)
name = getoffset(name, &stdoffset);
if (name == NULL)
return false;
charcnt = stdlen + 1;
if (sizeof sp->chars < charcnt)
return false;
/*
* This bit also differs from the IANA code, which doesn't make any
* attempt to avoid repetitive loadings of the TZDEFRULES zone.
*/
if (tzdefrules_loaded == 0)
{
if (tzload(TZDEFRULES, NULL, &tzdefrules_s, false) == 0)
tzdefrules_loaded = 1;
else
tzdefrules_loaded = -1;
}
load_ok = (tzdefrules_loaded > 0);
if (load_ok)
memcpy(sp, &tzdefrules_s, sizeof(struct state));
}
if (!load_ok)
sp->leapcnt = 0; /* so, we're off a little */
charcnt = stdlen + 1;
if (sizeof sp->chars < charcnt)
return false;
/*
* The IANA code always tries tzload(TZDEFRULES) here. We do not want to
* do that; it would be bad news in the lastditch case, where we can't
* assume pg_open_tzfile() is sane yet. Moreover, the only reason to do
* it unconditionally is to absorb the TZDEFRULES zone's leap second info,
* which we don't want to do anyway. Without that, we only need to load
* TZDEFRULES if the zone name specifies DST but doesn't incorporate a
* POSIX-style transition date rule, which is not a common case.
*/
sp->goback = sp->goahead = false; /* simulate failed tzload() */
sp->leapcnt = 0; /* intentionally assume no leap seconds */
if (*name != '\0')
{
if (*name == '<')
@ -996,8 +982,38 @@ tzparse(const char *name, struct state *sp, bool lastditch)
}
else
dstoffset = stdoffset - SECSPERHOUR;
if (*name == '\0' && !load_ok)
name = TZDEFRULESTRING;
if (*name == '\0')
{
/*
* The POSIX zone name does not provide a transition-date rule.
* Here we must load the TZDEFRULES zone, if possible, to serve as
* source data for the transition dates. Unlike the IANA code, we
* try to cache the data so it's only loaded once.
*/
if (tzdefrules_loaded == 0)
{
/* Allocate on first use */
if (tzdefrules_s == NULL)
tzdefrules_s = (struct state *) malloc(sizeof(struct state));
if (tzdefrules_s != NULL)
{
if (tzload(TZDEFRULES, NULL, tzdefrules_s, false) == 0)
tzdefrules_loaded = 1;
else
tzdefrules_loaded = -1;
/* In any case, we ignore leap-second data from the file */
tzdefrules_s->leapcnt = 0;
}
}
load_ok = (tzdefrules_loaded > 0);
if (load_ok)
memcpy(sp, tzdefrules_s, sizeof(struct state));
else
{
/* If we can't load TZDEFRULES, fall back to hard-wired rule */
name = TZDEFRULESTRING;
}
}
if (*name == ',' || *name == ';')
{
struct rule start;