Fix data checksum progress reporting docs

Started by Fujii Masao8 days ago7 messagesdocs
Jump to latest
#1Fujii Masao
masao.fujii@gmail.com

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

Attachments:

v1-0001-doc-Fix-data-checksum-progress-reporting-document.patchapplication/octet-stream; name=v1-0001-doc-Fix-data-checksum-progress-reporting-document.patchDownload+25-15
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Fujii Masao (#1)
Re: Fix data checksum progress reporting docs

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

#3Fujii Masao
masao.fujii@gmail.com
In reply to: Daniel Gustafsson (#2)
Re: Fix data checksum progress reporting docs

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

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Fujii Masao (#3)
Re: Fix data checksum progress reporting docs

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

Attachments:

0001-doc-Fix-glossary-entry-for-data-checksums-workers.patchapplication/octet-stream; name=0001-doc-Fix-glossary-entry-for-data-checksums-workers.patch; x-unix-mode=0644Download+12-21
#5Fujii Masao
masao.fujii@gmail.com
In reply to: Daniel Gustafsson (#4)
Re: Fix data checksum progress reporting docs

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

#6Daniel Gustafsson
daniel@yesql.se
In reply to: Fujii Masao (#5)
Re: Fix data checksum progress reporting docs

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

Attachments:

v2-0002-Add-a-comment-to-distinguish-backend-types.patchapplication/octet-stream; name=v2-0002-Add-a-comment-to-distinguish-backend-types.patch; x-unix-mode=0644Download+1-1
v2-0001-doc-Fix-glossary-entry-for-data-checksums-workers.patchapplication/octet-stream; name=v2-0001-doc-Fix-glossary-entry-for-data-checksums-workers.patch; x-unix-mode=0644Download+12-21
#7Fujii Masao
masao.fujii@gmail.com
In reply to: Daniel Gustafsson (#6)
Re: Fix data checksum progress reporting docs

Thanks for updating the patch!

On Fri, Jul 10, 2026 at 7:00 PM Daniel Gustafsson <daniel@yesql.se> 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