Re: Semantics of pg_file_settings view

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: Semantics of pg_file_settings view
Date: 2015-06-26 20:02:15
Message-ID: 5955.1435348935@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> The simplest change would be to move the whole thing to around line 208 in
> guc-file.l, just after the stanza that loads PG_AUTOCONF_FILENAME. Or you
> could argue that the approach is broken altogether, and that we should
> capture the data while we read the files, so that you have some useful
> data in the view even if ParseConfigFile later decides there's a syntax
> error. I'm actually thinking maybe we should flush that data-capturing
> logic altogether in favor of just not deleting the ConfigVariable list
> data structure, and generating the view directly from that data structure.
> (You could even imagine being able to finger syntax errors in the view
> that way, by having ParseConfigFile attach a dummy ConfigVariable entry
> when it bails out.)

While snouting around in the same area, I noticed that
ParseConfigDirectory() leaks memory: it neglects to pfree the file names
it's collected. I'm not sure that it's worth fixing in the back branches,
because you'd need to have SIGHUP'd the postmaster a few million times
before the leak would amount to anything worth noticing. However, this
does demonstrate that all the functionality we've loaded into the GUC code
of late is likely to have some memory leaks in it.

Combining this with my idea about preserving the ConfigVariable list,
I'm thinking that it would be a good idea for ProcessConfigFile() to
run in a context created for the purpose of processing the config files,
rather than blindly using the caller's context, which is likely to be
a process-lifespan context and thus not a good place to leak in.
We could keep this context around until the next SIGHUP event, so that
the ConfigVariable list remains available, and then destroy it and
replace it with the next ProcessConfigFile's instance of the context.
In this way, any leakage in the processing code could not accumulate
over multiple SIGHUPs, and so it would be certain to remain fairly
negligible.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-06-26 20:07:46 Re: Semantics of pg_file_settings view
Previous Message Robert Haas 2015-06-26 20:02:06 Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file