Re: per backend I/O statistics

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Nazir Bilal Yavuz <byavuz81@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-12-20T09:24:37Z
Lists: pgsql-hackers
On Fri, Dec 20, 2024 at 09:09:00AM +0000, Bertrand Drouvot wrote:
> Thanks for the report! I was not able able to reproduce (even with asan-enabled)
> but I think the test is wrong. Indeed the backend could fsync too during the test
> (see register_dirty_segment() and the case where the request queue is full).
> 
> I think the test should look like the attached instead, thoughts?

Hmm.  I cannot reproduce that here either.  FWIW, I use these settings
by default in my local builds, similarly to the CI which did not
complain:
ASAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0"
UBSAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:verbosity=2"
CFLAGS+="-fsanitize=undefined "
LDFLAGS+="-fsanitize=undefined "

grassquit uses something a bit different, which don't allow me to see
a problem with 027 even with fsync enabled:
ASAN_OPTIONS="print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0:detect_stack_use_after_return=0"
CFLAGS+="-fsanitize=address -fno-sanitize-recover=all "
LDFLAGS+="-fsanitize=address -fno-sanitize-recover=all "

Anyway, I was arriving at the same conclusion as you, because this is
just telling us that there could be "some" sync activity.  As
rewritten, this would just check that the after-the-fact counter is
never lower than the before-the-fact counter, which is still better
than removing the query.  I was thinking about doing the latter and
remove the query, but I'm also OK with what you have here, keeping the
query with a relaxed check.  I'll go adjust that in a bit..
--
Michael

Commits

  1. Rework handling of pending data for backend statistics

  2. Rename some pgstats callbacks related to flush of entries

  3. Relax regression test for fsync check of backend-level stats

  4. Add backend-level statistics to pgstats

  5. Extract logic filling pg_stat_get_io()'s tuplestore into its own routine

  6. Tweak some comments related to variable-numbered stats in pgstat.c