On Tue, Mar 04, 2025 at 08:48:28AM +0000, Bertrand Drouvot wrote:
> s/report/Report/ and s/system/system./? to be consistent with the other single
> line comments around.
Right.
> Yeah, I also think that's the right location.
We could be more optimal for the WAL receiver if we add more timestamp
calculations, I think, but that's a sensitive loop, and this is better
than no information anyway. If somebody has a better idea, feel free.
We could have an extra GUC to control that, but I'm feeling that we
should restructure the WAL receiver before that, perhaps leverage some
of its activity elsewhere (?).
> + /*
> + * Some BackendTypes only perform IO under IOOBJECT_WAL, hence exclude all
> + * rows for all the other objects for these.
> + */
> + if ((bktype == B_WAL_SUMMARIZER || bktype == B_WAL_RECEIVER ||
> + bktype == B_WAL_WRITER) && io_object != IOOBJECT_WAL)
> + return false;
>
> I think that makes sense and it removes 15 lines out of 86. This function is
> "hard" to read/parse from my point of view. Maybe we could re-write it in a
> simpler way but that's outside the purpose of this thread.
One thing I am planning to do here to improve the situation is the
addition of a regression test that queries pg_stat_io for all the
combinations of backend_type, object and contexts that are now
allowed, to keep track of the number of tuples we have.
> + WHERE backend_type = 'walsummarizer' AND object = 'wal'});
>
> The object = 'wal' is not needed (thanks to === 3), maybe we can remove this
> filtering?
>
> Also what about adding a test to check that sum(reads) is NULL where object != 'wal'?
Not sure it matters as long as we track the supported combinations.
We need something a bit more general here.
(I've actually found a different issue while looking at the WAL
receiver, which is a bit older than what we have here. Will post that
in a different thread with you in CC.)
--
Michael
