Re: Idea: closing the loop for "pg_ctl reload"

On February 19, 2015 08:26:45 PM Tom Lane wrote:
> Bug #12788 reminded me of a problem I think we've discussed before:
> if you use "pg_ctl reload" to trigger reload of the postmaster's
> config files, and there's something wrong with those files, there's
> no warning to you of that. The postmaster just bleats to its log and
> keeps running. If you don't think to look in the log to confirm
> successful reload, you're left with a time bomb: the next attempt
> to restart the postmaster will fail altogether because of the bad
> config file.
>
> I commented in the bug thread that there wasn't much that pg_ctl
> could do about this, but on reflection it seems like something we
> could fix, because pg_ctl must be able to read the postmaster.pid
> file in order to issue a reload signal. Consider a design like this:
>
> 1. Extend the definition of the postmaster.pid file to add another
> line, which will contain the time of the last postmaster configuration
> load attempt (might as well be a numeric Unix-style timestamp) and
> a boolean indication of whether that attempt succeeded or not.
>
> 2. Change pg_ctl so that after sending a reload signal, it sleeps
> for a second and checks for a change in the config load timestamp
> (repeat as necessary till timeout). Once it sees the timestamp
> change, it's in a position to report success or failure using the
> boolean. I think this should become the default behavior, though
> you could define -W to mean that there should be no wait or feedback.
>
> It's tempting to think of storing a whole error message rather than
> just a boolean, but I think that would complicate the pidfile definition
> undesirably. A warning to look in the postmaster log ought to be
> sufficient here.
>
> For extra credit, the pg_reload_conf() function could be made to behave
> similarly.
>
> I don't have the time to pursue this idea myself, but perhaps someone
> looking for a not-too-complicated project could take it on.
>
> regards, tom lane

Here's my first crack at this. Notes:
1/ I haven't done the '-W' flag Tom mentions yet.
2/ Likewise haven't touched pg_reload_conf()
3/ Design details: I introduced a new struct in pg_ctl containing the parsed-
out data from postmaster.pid, with functions to retrieve the data and to
dispose memory allocated for it. This made the change in do_reload fairly
straightforward. I think this struct can be used in other places in pg_ctl as
well, and potentially in other files as well.
4/ Question: Can I assume pg_malloc allocated memory is set to zero? If not,
is it OK to do a memset(..., 0, ...)? I don't have much experience on any of
the esoteric platforms pgsql supports...

jan