From: Álvaro Herrera <alvherre@kurilemu.de>
To: Manuel Reyes Bravo <manuelreyesbravo@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Fujii Masao <masao.fujii@gmail.com>
Cc: Adam Lee <adam8157@gmail.com>
Subject: Re: Add a test for index_rebuild_count of REPACK (CONCURRENTLY)
Date: Wed, 16 Sep 2026 16:30:16 +0200
Message-ID: <aqqmGGS6qqm5LI0Z@alvherre.pgsql> (raw)
In-Reply-To: <CA+bCEdCOiewao+v85Ptz1vXoPaRtvMZGLD-OQsyK2MF6yVhqyw@mail.gmail.com>
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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@kurilemu.de, manuelreyesbravo@gmail.com, pgsql-hackers@lists.postgresql.org, masao.fujii@gmail.com, adam8157@gmail.com
Subject: Re: Add a test for index_rebuild_count of REPACK (CONCURRENTLY)
In-Reply-To: <aqqmGGS6qqm5LI0Z@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox