Re: shared-memory based stats collector - v70

At Thu, 7 Apr 2022 16:37:51 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in
> Hi,
>
> On 2022-04-07 00:28:45 -0700, Andres Freund wrote:
> > I've gotten through the main commits (and then a fix for the apparently
> > inevitable bug that's immediately highlighted by the buildfarm), and the first
> > test. I'll call it a night now, and work on the other tests & docs tomorrow.
>
> I've gotten through the tests now. There's one known, not yet addressed, issue
> with the stats isolation test, see [1].
>
>
> Working on the docs. Found a few things worth raising:
>
> 1)
> Existing text:
> When the server shuts down cleanly, a permanent copy of the statistics
> data is stored in the <filename>pg_stat</filename> subdirectory, so that
> statistics can be retained across server restarts. When recovery is
> performed at server start (e.g., after immediate shutdown, server crash,
> and point-in-time recovery), all statistics counters are reset.
>
> The existing docs patch hadn't updated yet. My current edit is
>
> When the server shuts down cleanly, a permanent copy of the statistics
> data is stored in the <filename>pg_stat</filename> subdirectory, so that
> statistics can be retained across server restarts. When crash recovery is
> performed at server start (e.g., after immediate shutdown, server crash,
> and point-in-time recovery, but not when starting a standby that was shut
> down normally), all statistics counters are reset.
>
> but I'm not sure the parenthetical is easy enough to understand?

I can read it. But I'm not sure that the difference is obvious for
average users between "starting a standby from a basebackup" and
"starting a standby after a normal shutdown"..

Other than that, it might be easier to read if the additional part
were moved out to the end of the paragraph, prefixing with "Note:
". For example,

...
statistics can be retained across server restarts. When crash recovery is
performed at server start (e.g., after immediate shutdown, server crash,
and point-in-time recovery), all statistics counters are reset. Note that
crash recovery is not performed when starting a standby that was shut
down normally then all counters are retained.

> 2)
> The edit is not a problem, but it's hard to understand what the existing
> paragraph actually means?
>
> diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
> index 3247e056663..8bfb584b752 100644
> --- a/doc/src/sgml/high-availability.sgml
> +++ b/doc/src/sgml/high-availability.sgml
> @@ -2222,17 +2222,17 @@ HINT: You can then restart the server after making the necessary configuration
> ...
> <para>
> - The statistics collector is active during recovery. All scans, reads, blocks,
> + The cumulative statistics system is active during recovery. All scans, reads, blocks,
> index usage, etc., will be recorded normally on the standby. Replayed
> actions will not duplicate their effects on primary, so replaying an
> insert will not increment the Inserts column of pg_stat_user_tables.
> The stats file is deleted at the start of recovery, so stats from primary
> and standby will differ; this is considered a feature, not a bug.
> </para>
>
> <para>

Agreed partially. It's too detailed. It might not need to mention WAL
replay.

> I'll just commit the necessary bit, but we really ought to rephrase this.
>
>
>
>
> Greetings,
>
> Andres Freund
>
> [1] https://www.postgresql.org/message-id/20220407165709.jgdkrzqlkcwue6ko%40alap3.anarazel.de

--
Kyotaro Horiguchi
NTT Open Source Software Center