Re: pgsql: Improve performance of timezone loading, especially pg_timezone_

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-committers <pgsql-committers(at)postgresql(dot)org>
Subject: Re: pgsql: Improve performance of timezone loading, especially pg_timezone_
Date: 2017-05-07 09:03:54
Message-ID: CAKJS1f8z_cuEcYED4J0OcN0Fg9JO46Umyq4hysYcmPgGumDSMg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

, On 7 May 2017 at 17:19, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
>> On Wed, May 3, 2017 at 7:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Improve performance of timezone loading, especially pg_timezone_names view.
>
>> I am consistently getting below error after this commit in my Win7 machine:
>> running bootstrap script ... 2017-05-07 05:09:13.383 GMT [7908] LOG:
>> could not open directory "/installation/share/timezone/posixrules": No
>> such file or directory
>
>> This occurs both during initdb and server start.
>
> You sure it wasn't there before, too? That commit did not add any
> file reads that didn't happen before.

It didn't exist before either, but I don't think the error is what you
think. Please note the error message indicated it's looking for a
"directory" named posixrules, not the file.

The error seems to be due to this code:

if (canonname == NULL)
{
int result;

fullname[fullnamelen] = '/';
/* test above ensured this will fit: */
strcpy(fullname + fullnamelen + 1, name);
result = open(fullname, O_RDONLY | PG_BINARY, 0);
if (result >= 0)
return result;
/* If that didn't work, fall through to do it the hard way */
}

where we append the name, then try to open the file, but since the
file does not exist result is < 0 and we fall through to the for(;;)
loop and try to do scan_directory_ci() on the filename rather than the
directory.

On windows, where I also get the error, the stacktrace, as of
b58c433ef90b is as follows:

postgres.exe!scan_directory_ci(const char * dirname, const char *
fname, int fnamelen, char * canonname, int canonnamelen) Line 162 C
> postgres.exe!pg_open_tzfile(const char * name, char * canonname) Line 127 C
postgres.exe!tzloadbody(const char * name, char * canonname, state *
sp, char doextend, local_storage * lsp) Line 240 C
postgres.exe!tzload(const char * name, char * canonname, state * sp,
char doextend) Line 564 C
postgres.exe!tzparse(const char * name, state * sp, char lastditch) Line 959 C
postgres.exe!tzloadbody(const char * name, char * canonname, state *
sp, char doextend, local_storage * lsp) Line 421 C
postgres.exe!tzload(const char * name, char * canonname, state * sp,
char doextend) Line 564 C
postgres.exe!pg_tzset(const char * name) Line 291 C
postgres.exe!check_log_timezone(char * * newval, void * * extra,
GucSource source) Line 415 C
postgres.exe!call_string_check_hook(config_string * conf, char * *
newval, void * * extra, GucSource source, int elevel) Line 9905 C
postgres.exe!parse_and_validate_value(config_generic * record, const
char * name, const char * value, GucSource source, int elevel,
config_var_val * newval, void * * newextra) Line 5834 C
postgres.exe!set_config_option(const char * name, const char *
value, GucContext context, GucSource source, GucAction action, char
changeVal, int elevel, char is_reload) Line 6437 C
postgres.exe!ProcessConfigFileInternal(GucContext context, char
applySettings, int elevel) Line 441 C
postgres.exe!ProcessConfigFile(GucContext context) Line 158 C
postgres.exe!SelectConfigFiles(const char * userDoption, const char
* progname) Line 4810 C
postgres.exe!PostmasterMain(int argc, char * * argv) Line 854 C
postgres.exe!main(int argc, char * * argv) Line 229 C
postgres.exe!__tmainCRTStartup() Line 536 C
postgres.exe!mainCRTStartup() Line 377 C
kernel32.dll!00007ffdab5e13d2() Unknown
ntdll.dll!00007ffdadf754e4() Unknown

Perhaps we just need to put the NUL char back, to trim off the filename again:

/* If that didn't work, fall through to do it the hard way */
fullname[fullnamelen] = '\0';

but I've not yet looked into why the file is missing in the first place.

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message David Rowley 2017-05-07 09:33:20 Re: pgsql: Improve performance of timezone loading, especially pg_timezone_
Previous Message Tom Lane 2017-05-07 05:19:05 Re: pgsql: Improve performance of timezone loading, especially pg_timezone_