Re: pgsql: pgstat: add/extend tests for resetting various kinds of stats.

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: pgstat: add/extend tests for resetting various kinds of stats.
Date: 2022-09-16 18:39:04
Message-ID: 20220916183904.sss3vsau5tfgtx56@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Hi,

On 2022-09-15 15:59:52 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > pgstat: add/extend tests for resetting various kinds of stats.
>
> crake just failed [1] with what appears to be a race condition in
> a test case added by this commit:
>
> diff -U3 /home/andrew/bf/root/HEAD/pgsql/src/test/regress/expected/stats.out /home/andrew/bf/root/HEAD/pgsql.build/src/test/recovery/tmp_check/results/stats.out
> --- /home/andrew/bf/root/HEAD/pgsql/src/test/regress/expected/stats.out 2022-07-18 13:32:25.155847949 -0400
> +++ /home/andrew/bf/root/HEAD/pgsql.build/src/test/recovery/tmp_check/results/stats.out 2022-09-15 12:44:10.022975164 -0400
> @@ -563,7 +563,7 @@
> SELECT sessions > :db_stat_sessions FROM pg_stat_database WHERE datname = (SELECT current_database());
> ?column?
> ----------
> - t
> + f
> (1 row)
>
> -- Test pg_stat_bgwriter checkpointer-related stats, together with pg_stat_wal
>
> It seems likely to me that the n_sessions increment hasn't made it to shared
> memory because some background process happened to have a lock on the stats
> entry when we tried. Don't we need a pg_stat_force_next_flush() call before
> trying to inspect the sessions count?

Indeed. Pushed a fix. To see if there are other omissions, I made
pgstat_report_stat() never flush stats unless force is passed in. No other
failures.

> BTW, the header comment for pgstat_report_stat is badly in need of
> copy-editing.

Hm - you're talking about this sentence?

... Callers can use the returned time to set up
* a timeout after which to call pgstat_report_stat(true), but are not
* required to to do so.

Reads awkward. I'm struggling with the english language trying to improve it
:(.

"Callers can, but are not required to, use the returned time to set up a
timer, whose expiration should trigger a call to pgstat_report_stat(true)."?

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2022-09-16 20:17:48 Re: pgsql: pgstat: add/extend tests for resetting various kinds of stats.
Previous Message Andres Freund 2022-09-16 18:31:35 pgsql: Fix race condition in stats.sql added in 5264add7847