From: Justin Pryzby <pryzby@telsasoft.com>
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: when the startup process doesn't (logging startup delays)
Date: Sun, 25 Jul 2021 12:56:54 -0500
Message-ID: <20210725175654.GA23997@telsasoft.com> (raw)
In-Reply-To: <CAMm1aWb2OzbvOBxB-M+UUEXt0h2nmtNybo5x30b4vyCOLZB_dA@mail.gmail.com>
References: <20210609161918.GY16435@telsasoft.com>
<CAMm1aWZNkm0nbVzAsiqnNSpgRT3N0HjT_b30AmjsVLDzjHTgFA@mail.gmail.com>
<20210610100125.GE16435@telsasoft.com>
<CAMm1aWbj3G8YJohr+FUOt1R+vz+e5sGiGCoK5kSRCgmDj5k+0Q@mail.gmail.com>
<20210617125254.GB31772@telsasoft.com>
<CAMm1aWadG_xgncYxFt-GAqRXWL2WE-5bu3qdJtW6CYEwhw7QbQ@mail.gmail.com>
<CALj2ACUNaAQjLYnyQvG988d=Qzw5yQhaRLYN6ihaxZShJ6AeHg@mail.gmail.com>
<CAMm1aWZScAOM4CA60sS90y3qPNtZ75Wo+6mRUry06bVjfRqePg@mail.gmail.com>
<20210721131326.GA19620@telsasoft.com>
<CAMm1aWb2OzbvOBxB-M+UUEXt0h2nmtNybo5x30b4vyCOLZB_dA@mail.gmail.com>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, nitinjadhavpostgres@gmail.com, bharath.rupireddyforpostgres@gmail.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, andres@anarazel.de, sfrost@snowman.net, magnus@hagander.net, thomas.munro@gmail.com
Subject: Re: when the startup process doesn't (logging startup delays)
In-Reply-To: <20210725175654.GA23997@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox