Hi all, While doing some concurrency benchmarking with injection_points stats enabled in a server, I have been able to trigger an assertion failure in pgstat_begin_changecount_write(): #4 0x0000564917fdc816 in ExceptionalCondition (conditionName=0x7f60af0dc5c8 "(*cc & 1) == 0", fileName=0x7f60af0dc598 "../../../../src/include/utils/pgstat_internal.h", lineNumber=831) at assert.c:65 #5 0x00007f60af0da3ef in pgstat_begin_changecount_write (cc=0x7f60acbb8e10) at ../../../../src/include/utils/pgstat_internal.h:831 #6 0x00007f60af0db1d9 in pgstat_report_inj_fixed (numattach=0, numdetach=0, numrun=1, numcached=0, numloaded=0) at injection_stats_fixed.c:155 #7 0x00007f60af0d8b5c in injection_points_run (fcinfo=0x564931b66588) at injection_points.c:429
This can be reproduced as follows. First, postgresql.conf:
shared_preload_libraries = 'injection_points'
injection_points.stats = on
Then something like the following command:
$ cat create_inj.sql
\set id random(1,100000)
select injection_points_attach('popo:id', 'notice');
select injection_points_run('popo:id');
select injection_points_detach('popo:id');
$ pgbench -n -T 300 -f create_inj.sql -c 10
The failure is not surprising, because the stats reports can happen in
a concurrent fashion when a point is run for example, contrary to
other fixed-sized stats kind where the reports are only done by a
single process (archiver, bgwriter, checkpointer). So this is just a
matter of acquiring a lock that was forgotten, to make sure that the
changes are consistent. Far from critical as this is template code,
still embarrassing.
Thoughts or comments?
--
Michael
diff --git a/src/test/modules/injection_points/injection_stats_fixed.c b/src/test/modules/injection_points/injection_stats_fixed.c index bc54c79d190b..74c35fcbfa71 100644 --- a/src/test/modules/injection_points/injection_stats_fixed.c +++ b/src/test/modules/injection_points/injection_stats_fixed.c @@ -152,6 +152,8 @@ pgstat_report_inj_fixed(uint32 numattach, stats_shmem = pgstat_get_custom_shmem_data(PGSTAT_KIND_INJECTION_FIXED); + LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE); + pgstat_begin_changecount_write(&stats_shmem->changecount); stats_shmem->stats.numattach += numattach; stats_shmem->stats.numdetach += numdetach; @@ -159,6 +161,8 @@ pgstat_report_inj_fixed(uint32 numattach, stats_shmem->stats.numcached += numcached; stats_shmem->stats.numloaded += numloaded; pgstat_end_changecount_write(&stats_shmem->changecount); + + LWLockRelease(&stats_shmem->lock); } /*
signature.asc
Description: PGP signature
