Hi Álvaro,
Álvaro Herrera <alvherre@kurilemu.de> wrote:
> As I said in a reply to Fujii in the thread for the patch you replied to
> in pgsql-committers, I think we need to come up with a test framework
> specific to observing progress report counters. (Maybe, and I'm just
> braindumping here, have them in debug mode print out a line for each
> individual counter update that's made, so that a test file can
> observe/match those lines somehow).
Understood, and I should have read that thread first: Adam's original
patch had a test for this, and you and Fujii had already dropped it.
I would like to work on the framework for v20, if nobody else is. Some
facts that make it look tractable:
- Every write to st_progress_param goes through backend_progress.c
(pgstat_progress_update_param, _incr_param, _parallel_incr_param,
_update_multi_param, plus start/end_command). There are 163 calls in
23 files, and none of them writes the array directly, so one hook
there sees every update.
- There is precedent for the switch in the DEVELOPER_OPTIONS trace_*
settings (trace_locks, trace_notify, trace_sort, ...).
- As far as I can see, the only test that checks progress values for
their own sake is COPY's, from a trigger that reads
pg_stat_progress_copy during the insert. That needs user code running
inside the command, so it cannot reach VACUUM, ANALYZE, CREATE INDEX or
REPACK. Two recovery TAP tests poll pg_stat_progress_basebackup and
pg_stat_progress_vacuum, but only to know when to act.
Before writing anything, three questions, so that I build what you have
in mind:
1. A runtime developer setting (say trace_progress, like trace_notify)
or something compiled in only for debug builds (like LOCK_DEBUG
around trace_locks)?
2. Should tests read the lines from the server log in TAP tests, or
from the client with client_min_messages in the regression suite?
Counters such as blocks scanned vary between runs, so I assume a test
would match phases and selected counters rather than every line.
3. One line per call, or only when a value actually changes?
The first users would be VACUUM and REPACK, including the two
index_rebuild_count cases from this week.
Regards,
Manu
