Re: Add a test for index_rebuild_count of REPACK (CONCURRENTLY)

Hello Manu,

On 2026-Sep-16, Manuel Reyes Bravo wrote:

> Neither 4b445479f9e (TOAST index creation writing into
> index_rebuild_count) nor 0765b48874a (the concurrent path not counting
> its index builds) added a test, and both are easy to break again
> without anyone noticing, since nothing checks the values the progress
> views report.

I don't like this idea, because it adds no systematic mechanism to test
the progress-report feature as a whole. I don't see why REPACK should
be the place to start testing this. Also, injection points seem the
wrong tool for the job, even if you can achieve testing an increment of
a single progress counter within an existing test.

On the other hand, this proposed test uses the isolation framework,
which is by construction complicated enough. Not that this one
isolation test is particularly complicated; but other tests are, and we
do not benefit from added complexity that only supports testing a
feature unrelated to concurrency.

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). That's more work upfront, but it
can allow us systematically test all the counters in a coherent way.

Thanks for spending time on this,

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Doing what he did amounts to sticking his fingers under the hood of the
implementation; if he gets his fingers burnt, it's his problem." (Tom Lane)