From ee27366bf3f7f5fd6a9884462476b4240cdd5c9a Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sat, 21 Jan 2023 21:41:04 -0600 Subject: [PATCH 4/4] f! also assert that progress values don't go backwards and the total is constant See also: https://www.postgresql.org/message-id/CA%2BTgmoYSvEP3weQaCPGf6%2BDXLy2__JbJUYtoXyWP%3DqHcyGbihA%40mail.gmail.com --- src/backend/access/heap/vacuumlazy.c | 37 +++++++++ src/backend/commands/analyze.c | 10 ++- src/backend/storage/lmgr/lmgr.c | 24 +++--- src/backend/utils/activity/backend_progress.c | 77 +++++++++++++++++++ 4 files changed, 135 insertions(+), 13 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 8f14cf85f38..22661f6a292 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1042,6 +1042,13 @@ lazy_scan_heap(LVRelState *vacrel) /* Forget the LP_DEAD items that we just vacuumed */ dead_items->num_items = 0; + { + int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES}; + const int64 progress_vals[] = {0}; + + pgstat_progress_update_multi_param(1, progress_inds, progress_vals); + } + /* * Periodically perform FSM vacuuming to make newly-freed @@ -2199,6 +2206,13 @@ lazy_vacuum(LVRelState *vacrel) { Assert(!vacrel->do_index_cleanup); vacrel->dead_items->num_items = 0; + { + int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES}; + const int64 progress_vals[] = {0}; + + pgstat_progress_update_multi_param(1, progress_inds, progress_vals); + } + return; } @@ -2301,6 +2315,13 @@ lazy_vacuum(LVRelState *vacrel) * vacuum) */ vacrel->dead_items->num_items = 0; + + { + int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES}; + const int64 progress_vals[] = {0}; + + pgstat_progress_update_multi_param(1, progress_inds, progress_vals); + } } /* @@ -2414,12 +2435,23 @@ lazy_vacuum_heap_rel(LVRelState *vacrel) BlockNumber vacuumed_pages = 0; Buffer vmbuffer = InvalidBuffer; LVSavedErrInfo saved_err_info; +#if 0 + int const progress_inds[] = { + PROGRESS_VACUUM_PHASE, + PROGRESS_VACUUM_NUM_DEAD_TUPLES, + }; + const int64 progress_vals[] = { + PROGRESS_VACUUM_PHASE_VACUUM_HEAP, + 0, + }; +#endif Assert(vacrel->do_index_vacuuming); Assert(vacrel->do_index_cleanup); Assert(vacrel->num_index_scans > 0); /* Report that we are now vacuuming the heap */ + //pgstat_progress_update_multi_param(2, progress_inds, progress_vals); pgstat_progress_update_param(PROGRESS_VACUUM_PHASE, PROGRESS_VACUUM_PHASE_VACUUM_HEAP); @@ -3190,7 +3222,12 @@ dead_items_alloc(LVRelState *vacrel, int nworkers) dead_items = (VacDeadItems *) palloc(vac_max_items_to_alloc_size(max_items)); dead_items->max_items = max_items; dead_items->num_items = 0; + { + int const progress_inds[] = {PROGRESS_VACUUM_NUM_DEAD_TUPLES}; + const int64 progress_vals[] = {0}; + pgstat_progress_update_multi_param(1, progress_inds, progress_vals); + } vacrel->dead_items = dead_items; } diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index c86e690980e..96710b84558 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -1145,6 +1145,12 @@ acquire_sample_rows(Relation onerel, int elevel, TableScanDesc scan; BlockNumber nblocks; BlockNumber blksdone = 0; + int64 progress_vals[2] = {0}; + int const progress_inds[2] = { + PROGRESS_ANALYZE_BLOCKS_DONE, + PROGRESS_ANALYZE_BLOCKS_TOTAL + }; + #ifdef USE_PREFETCH int prefetch_maximum = 0; /* blocks to prefetch if enabled */ BlockSamplerData prefetch_bs; @@ -1169,8 +1175,8 @@ acquire_sample_rows(Relation onerel, int elevel, #endif /* Report sampling block numbers */ - pgstat_progress_update_param(PROGRESS_ANALYZE_BLOCKS_TOTAL, - nblocks); + progress_vals[1] = nblocks; + pgstat_progress_update_multi_param(2, progress_inds, progress_vals); /* Prepare for sampling rows */ reservoir_init_selection_state(&rstate, targrows); diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index ee9b89a6726..8666d850660 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -912,6 +912,15 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) int total = 0; int done = 0; + const int index[] = { + PROGRESS_WAITFOR_TOTAL, + PROGRESS_WAITFOR_DONE, + PROGRESS_WAITFOR_CURRENT_PID + }; + const int64 values[] = { + 0, 0, 0 + }; + /* Done if no locks to wait for */ if (locktags == NIL) return; @@ -930,7 +939,10 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) } if (progress) + { + pgstat_progress_update_multi_param(3, index, values); pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, total); + } /* * Note: GetLockConflicts() never reports our own xid, hence we need not @@ -960,19 +972,9 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) pgstat_progress_update_param(PROGRESS_WAITFOR_DONE, ++done); } } - if (progress) - { - const int index[] = { - PROGRESS_WAITFOR_TOTAL, - PROGRESS_WAITFOR_DONE, - PROGRESS_WAITFOR_CURRENT_PID - }; - const int64 values[] = { - 0, 0, 0 - }; + if (progress) pgstat_progress_update_multi_param(3, index, values); - } list_free_deep(holders); } diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c index d0566430fab..707ab929fac 100644 --- a/src/backend/utils/activity/backend_progress.c +++ b/src/backend/utils/activity/backend_progress.c @@ -116,6 +116,74 @@ pgstat_progress_asserts(void) } } +static void +pgstat_progress_assert_forward_progress(int command, int index, + int64 oldval, int64 newval) +{ + switch (command) + { + case PROGRESS_COMMAND_ANALYZE: + + /* + * phase goes backwards for inheritance tables, which are sampled + * twice + */ + if (index != PROGRESS_ANALYZE_CURRENT_CHILD_TABLE_RELID && + index != PROGRESS_ANALYZE_PHASE) + Assert(newval >= oldval); + if (index == PROGRESS_ANALYZE_BLOCKS_TOTAL || + index == PROGRESS_ANALYZE_EXT_STATS_TOTAL || + index == PROGRESS_ANALYZE_CHILD_TABLES_TOTAL) + Assert(oldval == 0); + break; + + case PROGRESS_COMMAND_CLUSTER: + if (index != PROGRESS_CLUSTER_INDEX_RELID) + Assert(newval >= oldval); + if (index == PROGRESS_CLUSTER_TOTAL_HEAP_BLKS) + Assert(oldval == 0); + break; + + case PROGRESS_COMMAND_CREATE_INDEX: + if (index != PROGRESS_CREATEIDX_INDEX_OID && + index != PROGRESS_CREATEIDX_SUBPHASE && + index != PROGRESS_WAITFOR_CURRENT_PID && + index != PROGRESS_CREATEIDX_ACCESS_METHOD_OID) + Assert(newval >= oldval); + + if (index == PROGRESS_CREATEIDX_TUPLES_TOTAL || + index == PROGRESS_CREATEIDX_PARTITIONS_TOTAL) + Assert(oldval == 0); + break; + + case PROGRESS_COMMAND_BASEBACKUP: + if (index == PROGRESS_BASEBACKUP_BACKUP_TOTAL && + oldval == 0 && newval == -1) + return; /* Do nothing: this is the initial "null" + * state before the size is estimated */ + Assert(newval >= oldval); + + if (index == PROGRESS_BASEBACKUP_BACKUP_TOTAL || + index == PROGRESS_BASEBACKUP_TBLSPC_TOTAL) + Assert(oldval == 0); + break; + + case PROGRESS_COMMAND_COPY: + Assert(newval >= oldval); + if (index == PROGRESS_COPY_BYTES_TOTAL) + Assert(oldval == 0); + break; + case PROGRESS_COMMAND_VACUUM: + Assert(newval >= oldval); + if (index == PROGRESS_VACUUM_TOTAL_HEAP_BLKS) + Assert(oldval == 0); + break; + + case PROGRESS_COMMAND_INVALID: + break; + } +} + /*----------- * pgstat_progress_update_param() - * @@ -132,6 +200,15 @@ pgstat_progress_update_param(int index, int64 val) if (!beentry || !pgstat_track_activities) return; + if (index != PROGRESS_SCAN_BLOCKS_DONE) + { + /* Check that progress does not go backwards */ + int64 oldval = beentry->st_progress_param[index]; + + pgstat_progress_assert_forward_progress(beentry->st_progress_command, + index, oldval, val); + } + PGSTAT_BEGIN_WRITE_ACTIVITY(beentry); beentry->st_progress_param[index] = val; PGSTAT_END_WRITE_ACTIVITY(beentry); -- 2.25.1