Hi, On Mon, Feb 03, 2025 at 01:07:26PM +0900, Michael Paquier wrote: > On Fri, Jan 31, 2025 at 11:29:31AM +0300, Nazir Bilal Yavuz wrote: > > On Wed, 29 Jan 2025 at 18:16, Bertrand Drouvot > > <bertranddrouvot...@gmail.com> wrote: > >> I think that's the main reason why ff99918c625 added this new GUC (looking > >> at > >> the commit message). I'd feel more comfortable if we keep it. > > > > As Michael suggested, I will run a couple of benchmarks to see the > > actual effect of this change. Then let's see if this affects anything. > > I've looked at bit at all that today, and something like the attached > is what seems like the best streamlined version to me for the main > feature. I am also planning to run some short benchmarks with > track_io_timing=on on HEAD and with the patch, then see the > difference, without any relationship to track_wal_io_timing.
Thanks! I've a few comments: === 1 + pgstat_count_io_op_time(IOOBJECT_WAL, IOCONTEXT_INIT, IOOP_WRITE, + io_start, 1, wal_segment_size); In case wal_init_zero is false, then we're only seeking to the end and write a solitary byte. Then, is reporting "wal_segment_size" correct? === 2 + /* + * Measure I/O timing to write WAL data, for pg_stat_wal + * and/or pg_stat_io. + */ + start = pgstat_prepare_io_time(track_wal_io_timing || track_io_timing); I think that makes sense done that way (as track_wal_io_timing does not have any effect in pgstat_count_io_op_time()). Nit: maybe change the order in the comment to reflect the code ordering? (I mean to say re-word to "for pg_stat_io and/or pg_stat_wal). The order is ok in issue_xlog_fsync() though. === 3 What about adding a message in the doc as mentioned in [1]? (I'd not be surprised if some people wonder why the "bytes" fields differ). === 4 pgstat_tracks_io_object() starts to be hard to read. I wonder if it could be simplified with switch but that could be done after this one goes in. === 5 I think this patch will help simplify the per-backend WAL related patch, that's nice. === 6 I'll also do some benchmark on my side. [1]: https://www.postgresql.org/message-id/Z5o9OQ0nwWD9tKTR%40ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
