> I also agree that this is the better place to do it. Hence modified
> the patch accordingly. The condition "!AmStartupProcess()" is added to
> differentiate whether the call is done from a startup process or some
> other process. Actually StartupXLOG() gets called in 2 places. one in
> StartupProcessMain() and the other in InitPostgres(). As the logging
> of the startup progress is required only during the startup process
> and not in the other cases,
The InitPostgres() case occurs when the server is started in bootstrap
mode (during initdb) or in single-user mode (postgres --single). I do
not see any reason why we shouldn't produce progress messages in at
least the latter case. I suspect that someone who is in the rather
desperate scenario of having to use single-user mode would really like
to know how long the server is going to take to start up.
Perhaps during initdb we don't want messages, but I'm not sure that we
need to do anything about that here. None of the messages that the
server normally produces show up when you run initdb, so I guess they
are getting redirected to /dev/null or something.
So I don't think that using AmStartupProcess() for this purpose is right.
On Tue, Sep 7, 2021 at 10:58 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Sep 03, 2021 at 09:23:27PM -0500, Justin Pryzby wrote:
> > On Fri, Sep 03, 2021 at 01:23:56PM +0530, Nitin Jadhav wrote:
> > > Please find the updated patch attached.
> >
> > Please check CA+TgmoZtbqxaOLdpNkBcDbz=41tWALA8kpH4M=RWtPYHC7-KNg@mail.gmail.com
> >
> > I agree with Robert that a standby server should not continuously show timing
> > messages for WAL replay.
>
> Clearly. This should be limited to crash recovery, and maybe there
> could be some checks to make sure that nothing is logged once a
> consistent point is reached. Honestly, I don't see why we should have
> a GUC for what's proposed here. A value too low would bloat the logs
> with entries that are not that meaningful. A value too large would
> just prevent access to some information wanted. Wouldn't it be better
> to just pick up a value like 10s or 20s?
>
> Looking at v13..
>
> + {"log_startup_progress_interval", PGC_SIGHUP, LOGGING_WHEN,
> + gettext_noop("Sets the time interval between each progress update "
> + "of the startup process."),
> + gettext_noop("0 logs all messages. -1 turns this feature off."),
> + GUC_UNIT_MS,
> The unit is incorrect here, as that would default to 10ms, contrary to
> what the documentation says about 10s.
> --
> Michael