Hi, On 2023-01-24 17:22:03 +0900, Kyotaro Horiguchi wrote: > Hello. > > At Thu, 19 Jan 2023 21:15:34 -0500, Melanie Plageman > <melanieplage...@gmail.com> wrote in > > Oh dear-- an extra FlushBuffer() snuck in there somehow. > > Removed it in attached v51. > > Also, I fixed an issue in my tablespace.sql updates > > I only looked 0002 and 0004. > (Sorry for the random order of the comment..) > > 0002: > > + Assert(pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType)); > > This is relatively complex checking. We already asserts-out increments > of invalid counters. Thus this is checking if some unrelated codes > clobbered them, which we do only when consistency is critical. Is > there any needs to do that here? I saw another occurance of the same > assertion.
I found it useful to find problems. > + no_temp_rel = bktype == B_AUTOVAC_LAUNCHER || bktype == B_BG_WRITER || > + bktype == B_CHECKPOINTER || bktype == B_AUTOVAC_WORKER || > + bktype == B_STANDALONE_BACKEND || bktype == B_STARTUP; > > I'm not sure I like to omit parentheses for such a long Boolean > expression on the right side. What parens would help? > + write_chunk_s(fpout, &pgStatLocal.snapshot.io); > + if (!read_chunk_s(fpin, &shmem->io.stats)) > > The names of the functions hardly make sense alone to me. How about > write_struct()/read_struct()? (I personally prefer to use > write_chunk() directly..) That's not related to this patch - there's several existing callers for it. And write_struct wouldn't be better imo, because it's not just for structs. > + PgStat_BktypeIO > > This patch abbreviates "backend" as "bk" but "be" is used in many > places. I think that naming should follow the predecessors. The precedence aren't consistent unfortunately :) > > + Number of read operations in units of <varname>op_bytes</varname>. > > I may be the only one who see the name as umbiguous between "total > number of handled bytes" and "bytes hadled at an operation". Can't it > be op_blocksize or just block_size? > > + b.io_object, > + b.io_context, No, block wouldn't be helpful - we'd like to use this for something that isn't uniform blocks. Greetings, Andres Freund
