Horton

Re: [Proposal] Adding callback support for custom statistics kinds

Sami Imseih <samimseih@gmail.com>

View thread
From: Sami Imseih <samimseih@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Chao Li <li.evan.chao@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-12-06T01:27:50Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix const correctness in pgstat data serialization callbacks

  2. test_custom_stats: Add tests with read/write of auxiliary data

  3. Allow cumulative statistics to read/write auxiliary data from/to disk

  4. test_custom_stats: Test module for custom cumulative statistics

  5. injection_points: Remove portions related to custom pgstats

  6. Rename routines for write/read of pgstats file

Attachments

> > However, as discussed off-list, I do think moving the custom kind tests from
> > injection points to the new test module is a prerequisite. I rather
> > not have us push a new test module that is doing duplicate work as the injection
> > stats tests.
> > I worked on this refactoring today and plan to have a patch ready for review
> > by tomorrow.
>
> Cool, thanks!

Attached is the new test module that replaces the custom statistics
tests currently in the injection points tests. Under test_custom_stats, there
are two separate modules: one for variable-amount stats and one for
fixed-amount stats. With this, we can completely remove the
stats-related tests and supporting code under
src/test/modules/injection_points/.

A few notes on the tests:

1. Variable stats: pgstat_drop_entry() and pgstat_fetch_entry() are
exercised here, addressing an earlier point raised in the thread.

2. Fixed-amount stats: I added specific tests for reset behavior; both
during crash recovery and during manual resets.

3. In test_custom_fixed_stats.c, you will see this comment:
```
/* see explanation above PgStatShared_Archiver for the reset protocol */
LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE);
```
This is intentional, as the reset protocol is documented at the
referenced location [0]. I wanted to call that out for the patch review.

Once this gets pushed, it will simplify the remaining work needed
for the remaining serialization callbacks work.

[0] https://github.com/postgres/postgres/blob/master/src/include/utils/pgstat_internal.h#L362-L382

--
Sami Imseih
Amazon Web Services (AWS)