Обсуждение: 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 https://postgr.es/m/CA%2BTgmoYqSF5sCNrgTom9r3Nh%3Dat4WmYFD%3DgsV-omStZ60S0ZUQ%40mail.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