| From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | "pgsql-docs(at)lists(dot)postgresql(dot)org" <pgsql-docs(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix data checksum progress reporting docs |
| Date: | 2026-07-10 10:00:33 |
| Message-ID: | A2DA6464-F27E-4545-9DE7-B61A6172DD18@yesql.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-docs |
> On 10 Jul 2026, at 03:13, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Thu, Jul 9, 2026 at 5:28 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>> You are absolutely right. And when looking at it I think we can simplify the
>> entry a bit as well to make it fit the style of the page better. What do you
>> think about the attached?
>
> Thanks for the patch! I like the overall approach.
> I just have a few minor comments.
>
> + A set of
> + <glossterm linkend="glossary-background-worker">background
> worker</glossterm>
> + processes which can enable, or disable, data checksums in a
>
> Wouldn't it be better to remove the two commas here?
Fixed.
> + running cluster. The process which coordinates the work is known as the
> + <firstterm>data checksums worker launcher</firstterm> and the process
>
> Only the glossary seems to use the term "data checksums worker launcher".
> Wouldn't "data checksums launcher" (i.e., dropping "worker") be a better name?
Fixed.
> + <firstterm>data checksums worker launcher</firstterm> and the process
> + which operate on the individual databases is known as the
> + <firstterm>data checksums worker</firstterm>.
>
> 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. Expanding the code to handle multiple parallel
workers was left as a future enhancement (it will perhaps require better IO
throttling etc).
> 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.
> 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?
--
Daniel Gustafsson
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0002-Add-a-comment-to-distinguish-backend-types.patch | application/octet-stream | 976 bytes |
| v2-0001-doc-Fix-glossary-entry-for-data-checksums-workers.patch | application/octet-stream | 3.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-07-10 13:29:47 | Re: Fix data checksum progress reporting docs |
| Previous Message | Shinya Kato | 2026-07-10 08:24:51 | Re: doc: Clarify ANALYZE VERBOSE output |