Re: when the startup process doesn't (logging startup delays)

On Thu, Aug 5, 2021 at 7:41 AM Nitin Jadhav
<nitinjadhavpostgres@gmail.com> wrote:
> This seems a little confusing. As we are setting
> last_startup_progress_timeout = now and then calling
> reset_startup_progress_timeout() which will calculate the next_time
> based on the value of last_startup_progress_timeout initially and
> checks whether next_timeout is less than now. It doesn't make sense to
> me. I feel we should calculate the next_timeout based on the time that
> it is supposed to fire next time. So we should set
> last_startup_progress_timeout = next_timeout after enabling the timer.
> Also I feel with the 2 functions mentioned above, we also need
> InitStartupProgress() which sets the initial value to
> startupProcessOpStartTime which is used to calculate the time
> difference and display in the logs. I could see those functions like
> below.
>
> InitStartupProgress(void)
> {
>     startupProcessOpStartTime = GetCurrentTimestamp();
>     ResetStartupProgressTimeout(startupProcessOpStartTime);
> }

This makes sense, but I think I'd like to have all the functions in
this patch use names_like_this() rather than NamesLikeThis().

> reset_startup_progress_timeout(TimeStampTz now)
> {
>   next_timeout = last_startup_progress_timeout + interval;
>   if (next_timeout < now)
>   {
>      // Either the timeout was processed so late that we missed an entire cycle,
>      // or the system clock was set backwards.
>      next_timeout = now + interval;
>   }
>   enable_timeout_at(next_timeout);
>   last_startup_progress_timeout = next_timeout;
> }

Hmm, yeah, that seems good, but given this change, maybe the variables
need a little renaming. Like change last_startup_progress_timeout to
scheduled_startup_progress_timeout, perhaps.

> startup_progress_timeout_has_expired()
> {
>    if (!startup_progress_timer_expired)
>      return;
>   now = GetCurrentTimestamp();
>   // compute timestamp difference based on startupProcessOpStartTime
>   reset_startup_progress_timeout(now);
> }

I guess this one needs to return a Boolean, actually.

-- 
Robert Haas
EDB: http://www.enterprisedb.com