Re: Move wal_buffers_full to WalUsage (and report it in pgss/explain)

On Tue, Feb 11, 2025 at 09:37:37AM +0000, Bertrand Drouvot wrote:
> While at it, adding 0004 to report wal_buffers_full in VACUUM/ANALYZE (VERBOSE).

Thanks for summarizing the history behind WalUsage and
wal_buffers_full. FWIW, 0001 that moves wal_buffers_full from
PgStat_PendingWalStats to WalUsage is going to make our lives much
easier for your pending patch to adds backend statistics for WAL. WAL
write/sync numbers/times will be covered in the backend stats by
pg_stat_io, allowing us to remove entirely the dependency to
PgStat_PendingWalStats.

I have been wondering for a bit if the comment at the top of WalUsage
should be updated, but the current description fits as well with the
follow-up patch series.

Anyway, if there are no objections, I am planning to apply this one to
lift this barrier and help with the follow-up work for the backend
stats. That's the mandatory piece to makes the backend WAL stats
integration easier.

0002, 0003 and 0004 are straight-forward follow-ups. It's IMO one of
these things where extra data is cheap to have access to, and can be
useful to be aware when distributed across multiple contexts like
queries, plans or even EXPLAIN. So no real objections here. If other
have comments, feel free.

show_wal_usage() should have its check on (usage->wal_buffers_full) in
explain.c, as Ilia has mentioned. It's true that you would not get a
(wal_buffers_full > 0) if at least the condition on wal_bytes is not
satisfied, but the addition makes sense on consistency grounds, at
least.

Agreed about the attribute ordering in pgss with everything associated
to WalUsage grouped together, btw.
--
Michael