enable_timeout_every() and fin_time
Hi,
I was looking using enable_timeout_every() in another place with Lukas
just now, and noticed the fin_time argument. It seems odd for an
interval firing interface to get an absolute timestamp as an
argument. The only in-tree user of enable_timeout_every() computes
fin_time explicitly using the interval time:
startup_progress_phase_start_time = GetCurrentTimestamp();
fin_time = TimestampTzPlusMilliseconds(startup_progress_phase_start_time,
log_startup_progress_interval);
enable_timeout_every(STARTUP_PROGRESS_TIMEOUT, fin_time,
log_startup_progress_interval);
In /messages/by-id/CA+TgmoYqSF5sCNrgTom9r3Nh=at4WmYFD=gsV-omStZ60S0ZUQ@mail.gmail.com
Robert said:
Apparently not, but here's a v2 anyway. In this version I made
enable_timeout_every() a three-argument function, so that the caller
can specify both the first time at which the timeout routine should be
called and the interval between them, instead of only the latter. That
seems to be more convenient for this use case, and is more powerful in
general.
What is the use case for an absolute start time plus a relative
interval?
ISTM that this will just lead to every caller ending up with a
calculation like the startup.c piece quoted above.
Greetings,
Andres Freund
On Sun, Jan 1, 2023 at 7:36 PM Andres Freund <andres@anarazel.de> wrote:
What is the use case for an absolute start time plus a relative
interval?
The code snippet that you indicate has the important side effect of
changing the global variable startup_progress_phase_start_time, which
is used by has_startup_progress_timeout_expired. Without the fin_time
argument, the timeout machinery would have to call
GetCurrentTimestamp() separately, and the caller wouldn't know what
answer it got. The result would be that the progress reports would
indicate an elapsed time relative to one timestamp, but the time at
which those progress reports were printed would be relative to a
slightly different timestamp.
Maybe nobody would notice such a minor discrepancy, but I wanted to avoid it.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2023-01-03 13:33:34 -0500, Robert Haas wrote:
On Sun, Jan 1, 2023 at 7:36 PM Andres Freund <andres@anarazel.de> wrote:
What is the use case for an absolute start time plus a relative
interval?The code snippet that you indicate has the important side effect of
changing the global variable startup_progress_phase_start_time, which
is used by has_startup_progress_timeout_expired. Without the fin_time
argument, the timeout machinery would have to call
GetCurrentTimestamp() separately, and the caller wouldn't know what
answer it got. The result would be that the progress reports would
indicate an elapsed time relative to one timestamp, but the time at
which those progress reports were printed would be relative to a
slightly different timestamp.
Maybe nobody would notice such a minor discrepancy, but I wanted to avoid it.
Doesn't that discrepancy already exist as the code stands, because
startup_progress_phase_start_time is also set in
has_startup_progress_timeout_expired()? I realize that was an example, but the
issue seems broader: After the first "firing", the next timeout will be
computed relative to an absolute time gathered in timestamp.c.
Greetings,
Andres Freund
On Tue, Jan 3, 2023 at 3:14 PM Andres Freund <andres@anarazel.de> wrote:
Doesn't that discrepancy already exist as the code stands, because
startup_progress_phase_start_time is also set in
has_startup_progress_timeout_expired()?
I don't think it is, actually.
I realize that was an example, but the
issue seems broader: After the first "firing", the next timeout will be
computed relative to an absolute time gathered in timestamp.c.
We're computing the time since the start of the current phase, not the
time since the last timeout. So I don't see how this is relevant.
--
Robert Haas
EDB: http://www.enterprisedb.com