> On 10 Jul 2026, at 15:29, Fujii Masao <[email protected]> wrote: > > Thanks for updating the patch!
Thanks for review, and apologies for the delay in getting back to this. > On Fri, Jul 10, 2026 at 7:00 PM Daniel Gustafsson <[email protected]> wrote: >>> Shouldn't "the process which operate" be either "the process which operates" >>> or "the processes which operate"? Since multiple data checksums workers >>> can run, the latter seems more appropriate. >> >> Currently the processing is limited to a single worker, so I think the >> proposed >> wording is more appropriate. > > So "operate" of "the process which operate" should be "operates" > (i.e., "s" is necessary for "operate")? Fixed. >>> While looking at the related code, I also found that >>> B_DATACHECKSUMSWORKER_LAUNCHER and B_DATACHECKSUMSWORKER_WORKER >>> were added as BackendType entries in miscadmin.h. Since those entries appear >>> under the comment for auxiliary processes, it could give the impression that >>> the data checksums launcher and workers are auxiliary processes. To avoid >>> that confusion, would it make sense to add a comment such as: >>> >>> ------------------------- >>> B_WAL_WRITER, >>> >>> + /* >>> + * XXXXXXXXXX >>> + */ >>> B_DATACHECKSUMSWORKER_LAUNCHER, >>> B_DATACHECKSUMSWORKER_WORKER, >>> ------------------------- >> >> Fair point, see attached. > > + /* Background worker processes */ > > Thanks for adding the comment! I'm a bit concerned that it's misleading, > since enum BackendType already contains background worker entries > such as B_BG_WORKER and B_SLOTSYNC_WORKER. So, how about making it > more specific? For example: > > ------------ > Data checksums processes are dynamic background workers, but they > use dedicated backend types for pgstat I/O accounting. > ------------ I like that version, added in the attached v3. >>> BTW, I'm also wondering why dedicated BackendType entries were added for >>> the data checksums processes, while other background workers don't have >>> their own BackendType values. >> >> They are need for the pgstat system though aren't they, or am I missing >> something? > > As I understand it, dedicated backend types are not inherently required > by the pgstat infrastructure. They are needed here so that pg_stat_io > can account for I/O performed by the data checksums launcher and > workers separately from generic background workers. Otherwise, > their I/O would simply be counted under the background worker backend > type, making it impossible to distinguish their I/O statistics. > Is this my understanding right?? Yes, your understanding matches mine, but I didn't explain it even close to as well as you did. -- Daniel Gustafsson
v3-0002-Add-a-comment-to-distinguish-backend-types.patch
Description: Binary data
v3-0001-doc-Fix-glossary-entry-for-data-checksums-workers.patch
Description: Binary data
