From: | alain radix <alain(dot)radix(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Requesting external_pid_file with postgres -C when not initialized lead to coredump |
Date: | 2016-06-22 16:12:10 |
Message-ID: | CA+YdpwyMhULqKJi=RfqiThRnubNUVcLFdvMwtQhWxvuvdDpS-A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Tom,
The actual fix do the job for preventing coredump.
Effectively, reporting "" instead of (null) would be more consistent with
the values found in pg_settings.
Another effect of the patch is that it become possible to start a cluster
with external_pid_file='' in postgresql.conf
It would have that same behavior as many other parameters.
I started a 9.6 beta with the following lines in postgresql.conf and the
only one that reported a problem is external_pid_file
external_pid_file = '' # write an extra PID file
unix_socket_group = '' # (change requires restart)
bonjour_name = '' # defaults to the computer name
ssl_ca_file = '' # (change requires restart)
ssl_crl_file = '' # (change requires restart)
krb_server_keyfile = ''
shared_preload_libraries = '' # (change requires restart)
synchronous_standby_names = '' # standby servers that provide sync rep
cluster_name = '' # added to process titles if
nonempty
default_tablespace = '' # a tablespace name, '' uses the default
temp_tablespaces = '' # a list of tablespace names, ''
uses
local_preload_libraries = ''
session_preload_libraries = ''
It seems to me that the general behavior it to consider an empty string as
an unset parameter in postgresql.conf, why should external_pid_file be an
exception ?
I would prefer postgresql -C to report an empty string to be consistent
with pg_settings in a backport of the fix.
I also would prefer external_pid_file to behave as other parameters in the
next versions.
Regards.
Alain
On 22 June 2016 at 17:24, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> alain radix <alain(dot)radix(at)gmail(dot)com> writes:
> > Here is a new patch with postmaster.c modification so that it check
> about a
> > empty string instead of a null pointer.
>
> Why should we adopt this, when there is already a better (more complete)
> fix in git?
>
> I'm not sold on the wisdom of modifying the semantics of
> external_pid_file, which is what this patch effectively does. I don't
> know if anything outside our core code looks at that variable, but if
> anything does, this would create issues for it.
>
> I'm even less sold on the wisdom of changing every other GUC that
> has a null bootstrap default, which is what we would also have to do
> in order to avoid the identical misbehavior for other -C cases.
>
> > The meaning is no more to avoid a core dump as you've done a change for
> > that but to have the same result with postgres -C as with a request to
> > pg_settings, an empty string when the parameter is not set.
>
> Well, maybe we should make -C print an empty string not "(nil)" for
> such cases. You're right that we don't make a distinction between
> NULL and "" in other code paths that display a string GUC, so doing
> so here is a bit inconsistent.
>
> regards, tom lane
>
--
Alain Radix
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2016-06-22 16:39:53 | Re: Question and suggestion about application binary compatibility policy |
Previous Message | Teodor Sigaev | 2016-06-22 16:01:00 | Re: Precedence of new phrase search tsquery operator |