Hi,
While reviewing the REPACK progress patch with this framework, I noticed
something the framework could not see, and the attached patch teaches it
to.
A progress phase lives in three places, and nothing keeps them in sync:
src/include/commands/progress.h the value
src/backend/catalog/system_views.sql the text the user sees
doc/src/sgml/monitoring.sgml the row in the phase table
The framework already derived the values from progress.h. It now also
reads the other two and compares them with what the commands actually
report: a phase value the view or the table does not know about fails
test, and a phase first reached out of the documented order is reported.
The second case is not hypothetical. Running VACUUM, ANALYZE, CLUSTER,
CREATE INDEX [CONCURRENTLY], REINDEX [CONCURRENTLY], REPACK and REPACK
(CONCURRENTLY) through it, nine of the ten come out consistent, and one
does not:
REPACK (CONCURRENTLY):
enabling logical decoding -> initializing logical decoding ->
seq scanning heap -> rebuilding index -> catch-up ->
swapping relation files -> performing final cleanup
REPACK went from "rebuilding index" to "catch-up", but the
documentation lists "catch-up" before "rebuilding index"
So the row order of the REPACK phase table is not the order the phases
happen in. build_new_index() sets REBUILD_INDEX and the concurrent path
reaches it before catch-up. This is pre-existing, not something the
in-flight REPACK progress patches introduce, but it is exactly the kind
of thing someone reads that table to find out.
The obvious trap here is a command that loops. VACUUM goes back to
"scanning heap" for every round of index vacuuming, which is legitimate
and documented, and a naive order check would flag it. Only the first
arrival at a phase is considered, so a return to a phase already seen is
not reported. The test covers that: with maintenance_work_mem at the
minimum it gets a real two-round VACUUM
scanning heap -> vacuuming indexes -> vacuuming heap ->
scanning heap -> vacuuming indexes -> vacuuming heap ->
cleaning up indexes -> truncating heap -> performing final cleanup
and reports nothing for it.
The out-of-order case is reported rather than failed, because it is a
documentation bug and I did not want a test that fails on master until
that table is reordered. If you would rather have it fail, that is a
one-line change and then the REPACK table needs fixing first.
The two paths come from the Makefile, as PROGRESS_H already did,
so a phase added in all three places keeps the check quiet, and a phase
added in two of them does not.
Regards,
Manu
