1
0
mirror of https://github.com/postgres/postgres.git synced 2026-10-23 13:29:09 +03:00

Fix data checksum progress counter initialization

pg_stat_progress_data_checksums uses -1 as a sentinel value that is
displayed as NULL for progress counters. However, after
pgstat_progress_start_command() initialized all progress counters to
zero, data checksum progress did not reset those counters to -1.
As a result, some counters could incorrectly appear as zero instead
of NULL. For example, workers could report zero database counters,
and the disabling launcher could report zero relation and block counters.

Also, blocks_done was not reset when a worker started processing
a new relation fork. As a result, it could temporarily exceed blocks_total
or report a stale value for an empty relation fork.

Fix this by initializing the data checksum progress counters to -1
when progress reporting starts for both launcher and worker processes.
Also reset blocks_done together with blocks_total when starting each
relation fork.

Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAHGQGwEOQyEzW2cqrHEzvwbcsAsuH8MEe7MMidFOFxECy0E1_Q@mail.gmail.com
Backpatch-through: 19
This commit is contained in:
Fujii Masao
2026-07-10 22:32:53 +09:00
parent 2bbec7c49a
commit 9d1d91a143
4 changed files with 50 additions and 18 deletions

View File

@@ -7945,8 +7945,8 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para> </para>
<para> <para>
The total number of databases which will be processed. Only the The total number of databases which will be processed. Only the
launcher process has this value set, the worker processes have this launcher process has this value set when enabling data checksums;
set to <literal>NULL</literal>. otherwise this is set to <literal>NULL</literal>.
</para> </para>
</entry> </entry>
</row> </row>
@@ -7958,8 +7958,8 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para> </para>
<para> <para>
The number of databases which have been processed. Only the launcher The number of databases which have been processed. Only the launcher
process has this value set, the worker processes have this set to process has this value set when enabling data checksums; otherwise
<literal>NULL</literal>. this is set to <literal>NULL</literal>.
</para> </para>
</entry> </entry>
</row> </row>

View File

@@ -1482,7 +1482,7 @@ CREATE VIEW pg_stat_progress_data_checksums AS
WHEN 4 THEN 'done' WHEN 4 THEN 'done'
END AS phase, END AS phase,
CASE S.param2 WHEN -1 THEN NULL ELSE S.param2 END AS databases_total, CASE S.param2 WHEN -1 THEN NULL ELSE S.param2 END AS databases_total,
S.param3 AS databases_done, CASE S.param3 WHEN -1 THEN NULL ELSE S.param3 END AS databases_done,
CASE S.param4 WHEN -1 THEN NULL ELSE S.param4 END AS relations_total, CASE S.param4 WHEN -1 THEN NULL ELSE S.param4 END AS relations_total,
CASE S.param5 WHEN -1 THEN NULL ELSE S.param5 END AS relations_done, CASE S.param5 WHEN -1 THEN NULL ELSE S.param5 END AS relations_done,
CASE S.param6 WHEN -1 THEN NULL ELSE S.param6 END AS blocks_total, CASE S.param6 WHEN -1 THEN NULL ELSE S.param6 END AS blocks_total,

View File

@@ -391,6 +391,7 @@ static void FreeDatabaseList(List *dblist);
static DataChecksumsWorkerResult ProcessDatabase(DataChecksumsWorkerDatabase *db); static DataChecksumsWorkerResult ProcessDatabase(DataChecksumsWorkerDatabase *db);
static bool ProcessAllDatabases(void); static bool ProcessAllDatabases(void);
static bool ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrategy strategy); static bool ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrategy strategy);
static void ResetDataChecksumsProgressCounters(void);
static void launcher_cancel_handler(SIGNAL_ARGS); static void launcher_cancel_handler(SIGNAL_ARGS);
static void WaitForAllTransactionsToFinish(void); static void WaitForAllTransactionsToFinish(void);
@@ -698,7 +699,19 @@ ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrateg
snprintf(activity, sizeof(activity) - 1, "processing: %s.%s (%s, %u blocks)", snprintf(activity, sizeof(activity) - 1, "processing: %s.%s (%s, %u blocks)",
(relns ? relns : ""), RelationGetRelationName(reln), forkNames[forkNum], numblocks); (relns ? relns : ""), RelationGetRelationName(reln), forkNames[forkNum], numblocks);
pgstat_report_activity(STATE_RUNNING, activity); pgstat_report_activity(STATE_RUNNING, activity);
pgstat_progress_update_param(PROGRESS_DATACHECKSUMS_BLOCKS_TOTAL, numblocks); {
const int index[] = {
PROGRESS_DATACHECKSUMS_BLOCKS_TOTAL,
PROGRESS_DATACHECKSUMS_BLOCKS_DONE
};
int64 vals[2];
vals[0] = numblocks;
vals[1] = 0;
pgstat_progress_update_multi_param(2, index, vals);
}
if (relns) if (relns)
pfree(relns); pfree(relns);
@@ -764,6 +777,29 @@ ProcessSingleRelationFork(Relation reln, ForkNumber forkNum, BufferAccessStrateg
return true; return true;
} }
/*
* Initialize all data checksum progress counters to be displayed as NULL.
*/
static void
ResetDataChecksumsProgressCounters(void)
{
const int index[] = {
PROGRESS_DATACHECKSUMS_DBS_TOTAL,
PROGRESS_DATACHECKSUMS_DBS_DONE,
PROGRESS_DATACHECKSUMS_RELS_TOTAL,
PROGRESS_DATACHECKSUMS_RELS_DONE,
PROGRESS_DATACHECKSUMS_BLOCKS_TOTAL,
PROGRESS_DATACHECKSUMS_BLOCKS_DONE,
};
int64 vals[lengthof(index)];
for (int i = 0; i < lengthof(index); i++)
vals[i] = -1;
pgstat_progress_update_multi_param(lengthof(index), index, vals);
}
/* /*
* ProcessSingleRelationByOid * ProcessSingleRelationByOid
* Process a single relation based on oid. * Process a single relation based on oid.
@@ -1142,6 +1178,7 @@ again:
pgstat_progress_start_command(PROGRESS_COMMAND_DATACHECKSUMS, pgstat_progress_start_command(PROGRESS_COMMAND_DATACHECKSUMS,
InvalidOid); InvalidOid);
ResetDataChecksumsProgressCounters();
if (operation == ENABLE_DATACHECKSUMS) if (operation == ENABLE_DATACHECKSUMS)
{ {
@@ -1269,23 +1306,14 @@ ProcessAllDatabases(void)
const int index[] = { const int index[] = {
PROGRESS_DATACHECKSUMS_DBS_TOTAL, PROGRESS_DATACHECKSUMS_DBS_TOTAL,
PROGRESS_DATACHECKSUMS_DBS_DONE, PROGRESS_DATACHECKSUMS_DBS_DONE,
PROGRESS_DATACHECKSUMS_RELS_TOTAL,
PROGRESS_DATACHECKSUMS_RELS_DONE,
PROGRESS_DATACHECKSUMS_BLOCKS_TOTAL,
PROGRESS_DATACHECKSUMS_BLOCKS_DONE,
}; };
int64 vals[6]; int64 vals[2];
vals[0] = list_length(DatabaseList); vals[0] = list_length(DatabaseList);
vals[1] = 0; vals[1] = 0;
/* translated to NULL */
vals[2] = -1;
vals[3] = -1;
vals[4] = -1;
vals[5] = -1;
pgstat_progress_update_multi_param(6, index, vals); pgstat_progress_update_multi_param(2, index, vals);
} }
foreach_ptr(DataChecksumsWorkerDatabase, db, DatabaseList) foreach_ptr(DataChecksumsWorkerDatabase, db, DatabaseList)
@@ -1581,6 +1609,7 @@ DataChecksumsWorkerMain(Datum arg)
/* worker will have a separate entry in pg_stat_progress_data_checksums */ /* worker will have a separate entry in pg_stat_progress_data_checksums */
pgstat_progress_start_command(PROGRESS_COMMAND_DATACHECKSUMS, pgstat_progress_start_command(PROGRESS_COMMAND_DATACHECKSUMS,
InvalidOid); InvalidOid);
ResetDataChecksumsProgressCounters();
/* /*
* Get a list of all temp tables present as we start in this database. We * Get a list of all temp tables present as we start in this database. We

View File

@@ -2115,7 +2115,10 @@ pg_stat_progress_data_checksums| SELECT s.pid,
WHEN '-1'::integer THEN NULL::bigint WHEN '-1'::integer THEN NULL::bigint
ELSE s.param2 ELSE s.param2
END AS databases_total, END AS databases_total,
s.param3 AS databases_done, CASE s.param3
WHEN '-1'::integer THEN NULL::bigint
ELSE s.param3
END AS databases_done,
CASE s.param4 CASE s.param4
WHEN '-1'::integer THEN NULL::bigint WHEN '-1'::integer THEN NULL::bigint
ELSE s.param4 ELSE s.param4