On March 3, 2015 06:34:33 PM Jim Nasby wrote:
> On 3/3/15 5:24 PM, Jan de Visser wrote:> On March 3, 2015 04:57:58 PM
> Jim Nasby wrote:
> >> On 3/3/15 11:48 AM, Andres Freund wrote:
> >>> I'm saying that you'll need a way to notice that a reload was
> processed
> >> > or not. And that can't really be the message itself, there has to be
> >> > some other field; like the timestamp Tom proposes.
> >>
> >> Ahh, right. We should make sure we don't go brain-dead if the system
> >> clock moves backwards. I assume we couldn't just fstat the file...
> >
> > The timestamp field is already there (in my patch). It gets populated
> when the
> > server starts and repopulated by SIGHUP_handler. It's the only timestamp
> > pg_ctl pays attention to.
>
> What happens if someone does a reload sometime in the future (perhaps
> because they've messed with the system clock for test purposes). Do we
> still detect a new reload?
Good point, and I had that scenario in the back of my head. What I do right
now is read the 'last reload' timestamp from postmaster.pid (which can be the
same as the startup time, since I make postmaster write an initial timestamp
when it starts), send the SIGHUP, and wait until I read a later one or until
timeout. What I could do is wait until I read a *different* one, and not just
a later one. In that case you're only SOL if you manage to time it just so
that your new server time is *exactly* the same as your old server time when
you did your last reload (or startup). But even in that case it would time out
and recommend you check the log.
That whole error message thing has one gotcha BTW; it's not hard, it's just
that I have to find a way to make it bubble up from guc_file.l. Triggering an
error was just changing the return value from void to bool, but returning the
error string would mean returning a char buffer which would then need to be
freed by other callers of ProcessConfig etc etc.
* /me scratches head
But I could just rename the current ProcessConfig, make it return a buffer,
and have a *new*, void, ProcessConfig which calls the renamed function and
just discards the result.
As an aside: I always found it interesting how feature threads "explode"
around here. But it figures if even something as "simple" as this gets such
detailed input. Definitely something to get used to...
