On Wed, Jun 23, 2021 at 09:39:37AM +0900, Michael Paquier wrote:
> Perhaps you are right to keep it simple.  If people would like to
> document that more precisely, it could always be changed if
> necessary.  What you have here sounds pretty much right to me.

So, I was looking at this one today, and got confused with the name of
the counters once the patch was in place as this leads to having
things like "blocks" and "total_blocks_modified", which is a bit
confusing as "blocks" stands for the number of blocks scanned,
including new pages.  I have simply suffixed "files" and "blocks" with
"_scanned" to be more consistent with the new counters that are named
"_written", giving the attached.  We still need to have the per-file
counter in scan_file() with the global counters updated at the end of
a file scan for the sake of the file counter, of course.

Does that look fine to you?
--
Michael
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 831cf42d3a..a13cf22b57 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -31,8 +31,10 @@
 #include "storage/checksum_impl.h"
 
 
-static int64 files = 0;
-static int64 blocks = 0;
+static int64 files_scanned = 0;
+static int64 files_written = 0;
+static int64 blocks_scanned = 0;
+static int64 blocks_written = 0;
 static int64 badblocks = 0;
 static ControlFileData *ControlFile;
 
@@ -195,6 +197,7 @@ scan_file(const char *fn, BlockNumber segmentno)
 	int			f;
 	BlockNumber blockno;
 	int			flags;
+	int64		blocks_written_in_file = 0;
 
 	Assert(mode == PG_MODE_ENABLE ||
 		   mode == PG_MODE_CHECK);
@@ -208,7 +211,7 @@ scan_file(const char *fn, BlockNumber segmentno)
 		exit(1);
 	}
 
-	files++;
+	files_scanned++;
 
 	for (blockno = 0;; blockno++)
 	{
@@ -227,7 +230,7 @@ scan_file(const char *fn, BlockNumber segmentno)
 							 blockno, fn, r, BLCKSZ);
 			exit(1);
 		}
-		blocks++;
+		blocks_scanned++;
 
 		/*
 		 * Since the file size is counted as total_size for progress status
@@ -256,6 +259,12 @@ scan_file(const char *fn, BlockNumber segmentno)
 		{
 			int			w;
 
+			/* Do not rewrite if the checksum is already set to the expected value */
+			if (header->pd_checksum == csum)
+				continue;
+
+			blocks_written_in_file++;
+
 			/* Set checksum in page header */
 			header->pd_checksum = csum;
 
@@ -292,6 +301,13 @@ scan_file(const char *fn, BlockNumber segmentno)
 			pg_log_info("checksums enabled in file \"%s\"", fn);
 	}
 
+	/* Update write counters if any write activity has happened */
+	if (blocks_written_in_file > 0)
+	{
+		files_written++;
+		blocks_written += blocks_written_in_file;
+	}
+
 	close(f);
 }
 
@@ -637,8 +653,8 @@ main(int argc, char *argv[])
 			progress_report(true);
 
 		printf(_("Checksum operation completed\n"));
-		printf(_("Files scanned:  %s\n"), psprintf(INT64_FORMAT, files));
-		printf(_("Blocks scanned: %s\n"), psprintf(INT64_FORMAT, blocks));
+		printf(_("Files scanned:   %s\n"), psprintf(INT64_FORMAT, files_scanned));
+		printf(_("Blocks scanned:  %s\n"), psprintf(INT64_FORMAT, blocks_scanned));
 		if (mode == PG_MODE_CHECK)
 		{
 			printf(_("Bad checksums:  %s\n"), psprintf(INT64_FORMAT, badblocks));
@@ -647,6 +663,11 @@ main(int argc, char *argv[])
 			if (badblocks > 0)
 				exit(1);
 		}
+		else if (mode == PG_MODE_ENABLE)
+		{
+			printf(_("Files written:  %s\n"), psprintf(INT64_FORMAT, files_written));
+			printf(_("Blocks written: %s\n"), psprintf(INT64_FORMAT, blocks_written));
+		}
 	}
 
 	/*
diff --git a/doc/src/sgml/ref/pg_checksums.sgml b/doc/src/sgml/ref/pg_checksums.sgml
index c84bc5c5b2..6a6aebed51 100644
--- a/doc/src/sgml/ref/pg_checksums.sgml
+++ b/doc/src/sgml/ref/pg_checksums.sgml
@@ -47,7 +47,8 @@ PostgreSQL documentation
 
   <para>
    When verifying checksums, every file in the cluster is scanned. When
-   enabling checksums, every file in the cluster is rewritten in-place.
+   enabling checksums, each relation file block with a changed checksum is 
+   rewritten in-place.
    Disabling checksums only updates the file <filename>pg_control</filename>.
   </para>
  </refsect1>

Attachment: signature.asc
Description: PGP signature

Reply via email to