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
signature.asc
Description: PGP signature