Horton

Thread

  1. Fix data checksum progress reporting docs

    Fujii Masao <masao.fujii@gmail.com> — 2026-07-08T12:23:32Z

    Hi,

While trying the new features in v19, I found a few documentation
issues with pg_stat_progress_data_checksums:

- pg_stat_progress_data_checksums has its own section, but is missing
   from the progress view summary table.

- The general progress reporting overview does not mention online data
   checksum operations.

- The section describes only checksum enabling, even though the view
   also reports checksum disabling.

- The progress counter columns are documented as integer, but are
   actually bigint.

The attached patch updates monitoring.sgml to address these issues.

Regards,

-- 
Fujii Masao

  2. Re: Fix data checksum progress reporting docs

    Daniel Gustafsson <daniel@yesql.se> — 2026-07-08T13:24:25Z

    > On 8 Jul 2026, at 14:23, Fujii Masao <masao.fujii@gmail.com> wrote:

> While trying the new features in v19, I found a few documentation
> issues with pg_stat_progress_data_checksums:

Thanks a lot for post-commit review!

> - pg_stat_progress_data_checksums has its own section, but is missing
>   from the progress view summary table.
> 
> - The general progress reporting overview does not mention online data
>   checksum operations.
> 
> - The section describes only checksum enabling, even though the view
>   also reports checksum disabling.
> 
> - The progress counter columns are documented as integer, but are
>   actually bigint.

The proposed changes all look good to me, thanks!

--
Daniel Gustafsson

  3. Re: Fix data checksum progress reporting docs

    Fujii Masao <masao.fujii@gmail.com> — 2026-07-09T00:17:46Z

    On Wed, Jul 8, 2026 at 10:24 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> The proposed changes all look good to me, thanks!

Thanks for the review! I've pushed the patch.

BTW, this is a separate issue, but also related to the data checksums docs,
I found that the glossary entry for "Auxiliary process" includes
the data checksums worker and data checksums worker launcher.
However, their own glossary entries describe them as background workers,
and they are implemented as dynamic background workers.

This seems inconsistent. Other dynamic background workers, such as
logical replication apply workers, are not listed as auxiliary processes.
I don't see why the data checksums worker and launcher should be treated
differently.

Wouldn't it be better to remove the data checksums worker and data
checksums worker launcher from the "Auxiliary process" entry?

Regards,

-- 
Fujii Masao

  4. Re: Fix data checksum progress reporting docs

    Daniel Gustafsson <daniel@yesql.se> — 2026-07-09T08:28:18Z

    > On 9 Jul 2026, at 02:17, Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> On Wed, Jul 8, 2026 at 10:24 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>> The proposed changes all look good to me, thanks!
> 
> Thanks for the review! I've pushed the patch.
> 
> BTW, this is a separate issue, but also related to the data checksums docs,
> I found that the glossary entry for "Auxiliary process" includes
> the data checksums worker and data checksums worker launcher.
> However, their own glossary entries describe them as background workers,
> and they are implemented as dynamic background workers.
> 
> This seems inconsistent. Other dynamic background workers, such as
> logical replication apply workers, are not listed as auxiliary processes.
> I don't see why the data checksums worker and launcher should be treated
> differently.
> 
> Wouldn't it be better to remove the data checksums worker and data
> checksums worker launcher from the "Auxiliary process" entry?

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?

--
Daniel Gustafsson

  5. Re: Fix data checksum progress reporting docs

    Fujii Masao <masao.fujii@gmail.com> — 2026-07-10T01:13:21Z

    On Thu, Jul 9, 2026 at 5:28 PM Daniel Gustafsson <daniel@yesql.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?


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


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


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


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.

Regards,

-- 
Fujii Masao

  6. Re: Fix data checksum progress reporting docs

    Daniel Gustafsson <daniel@yesql.se> — 2026-07-10T10:00:33Z

    > On 10 Jul 2026, at 03:13, Fujii Masao <masao.fujii@gmail.com> wrote:
> 
> On Thu, Jul 9, 2026 at 5:28 PM Daniel Gustafsson <daniel@yesql.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