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

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>
Cc: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Stephen Frost <sfrost(at)snowman(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: when the startup process doesn't (logging startup delays)
Date: 2021-07-25 17:56:54
Message-ID: 20210725175654.GA23997@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 23, 2021 at 04:09:47PM +0530, Nitin Jadhav wrote:
> > I think walkdir() should only call LogStartupProgress(FSYNC_IN_PROGRESS, path);
> > when action == datadir_fsync_fname.
>
> I agree and fixed it.

I saw that you fixed it by calling InitStartupProgress() after the walkdir()
calls which do pre_sync_fname. So then walkdir is calling
LogStartupProgress(STARTUP_PROCESS_OP_FSYNC) even when it's not doing fsync,
and then LogStartupProgress() is returning because !AmStartupProcess().

That seems indirect, fragile, and confusing. I suggest that walkdir() should
take an argument for which operation to pass to LogStartupProgress(). You can
pass a special enum for cases where nothing should be logged, like
STARTUP_PROCESS_OP_NONE.

On Wed, Jul 21, 2021 at 04:47:32PM +0530, Bharath Rupireddy wrote:
> 1) I still don't see the need for two functions LogStartupProgress and
> LogStartupProgressComplete. Most of the code is duplicate. I think we
> can just do it with a single function something like [1]:

I agree that one function can do this more succinctly. I think it's best to
use a separate enum value for START operations and END operations.

switch(operation)
{
case STARTUP_PROCESS_OP_SYNCFS_START:
ereport(...);
break;

case STARTUP_PROCESS_OP_SYNCFS_END:
ereport(...);
break;

case STARTUP_PROCESS_OP_FSYNC_START:
ereport(...);
break;

case STARTUP_PROCESS_OP_FSYNC_END:
ereport(...);
break;

...

--
Justin

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-07-25 18:53:16 Re: Removing "long int"-related limit on hash table sizes
Previous Message Ranier Vilela 2021-07-25 17:19:26 Re: Removing "long int"-related limit on hash table sizes