public inbox for [email protected]  

help / color / mirror / Atom feed

From: Fujii Masao <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Fix data checksum progress reporting docs
Date: Fri, 10 Jul 2026 22:29:47 +0900
Message-ID: <CAHGQGwGQsMnUBSSTawOtri9qr3wi5fE8V2+GcZw-rT=Vg33wQA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAHGQGwHJHJYAkYZBi3_O13np-Rou9UL637=hB3Y_-qdCgcZn-w@mail.gmail.com>
	<[email protected]>
	<CAHGQGwEv-C9ia+rBYyePzO8F=5FVvS412ZqcOupazuOb5RafNg@mail.gmail.com>
	<[email protected]>
	<CAHGQGwFsBjQs2fv7b72hxzGV_fJMh6LAg4E83pNfDOu1jVgWCA@mail.gmail.com>
	<[email protected]>

Thanks for updating the patch!

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")?


> > 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.
    ------------


> > 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??

Regards,

-- 
Fujii Masao

view thread (7+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix data checksum progress reporting docs
  In-Reply-To: <CAHGQGwGQsMnUBSSTawOtri9qr3wi5fE8V2+GcZw-rT=Vg33wQA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox