Re: bug: ANALYZE progress report with inheritance tables

On 22/01/2023 18:23, Justin Pryzby wrote:
> pg_stat_progress_analyze was added in v13 (a166d408e).
> 
> For tables with inheritance children, do_analyze_rel() and
> acquire_sample_rows() are called twice.  The first time through,
> pgstat_progress_start_command() has memset() the progress array to zero.
> 
> But the 2nd time, ANALYZE_BLOCKS_DONE is already set from the previous
> call, and BLOCKS_TOTAL can be set to some lower value (and in any case a
> value unrelated to the pre-existing value of BLOCKS_DONE).  So the
> progress report briefly shows a bogus combination of values and, with
> these assertions, fails regression tests in master and v13, unless
> BLOCKS_DONE is first zeroed.

Good catch!

I think the counts need do be reset even earlier, in 
acquire_inherited_sample_rows(), at the same time that we update 
PROGRESS_ANALYZE_CURRENT_CHILD_TABLE_RELID. See attached patch. 
Otherwise, there's a brief moment where we have already updated the 
child table ID, but the PROGRESS_ANALYZE_BLOCKS_TOTAL 
PROGRESS_ANALYZE_BLOCKS_DONE still show the counts from the previous 
child table. And if it's a foreign table, the FDW's sampling function 
might not update the progress report at all, in which case the old 
values will be displayed until the table is fully processed.

I appreciate the assertions you added, that made it easy to reproduce 
the problem. I'm inclined to not commit that though. It seems like a 
modularity violation for the code in backend_progress.c to have such 
intimate knowledge of what the different counters mean.

-- 
Heikki Linnakangas
Neon (https://neon.tech)