Re: Adding REPACK [concurrently]

On 2026-May-08, Amit Kapila wrote:

> On Wed, May 6, 2026 at 1:55 PM Antonin Houska <ah@cybertec.at> wrote:

> > One idea occurred to me yet, effectively it's just a cleanup. Part of it was
> > already proposed [1].

Hmm, I think this cleanup makes sense.  If I apply the test patches
(0001 and 0002 here), they fail almost immediately; but after applying
0003 all is again well.  I think these tests are a good thing to have in
the tree, even if we end up reverting db-specific snapshots later.

After some back and forth, I modified the tests slightly so that
the search PG_TEST_EXTRA for the string "stress_concurrently=N".  The N
can be changed so that the tests run for longer; if not given, it's
taken as 1, and the tests run for around 6 seconds (so N=10 means runs
for a minute).  I think this is a convenient gadget for other tests of
this kind on CONCURRENTLY commands, such as the one proposed for CIC
elsewhere.

As written, these tests would run nowhere until we add that string in
some buildfarm animal.  I debated with myself whether to assume N=1 when
the string is not given.  I still think that's a good idea but perhaps
we should have something to prevent it from running by default when
under Valgrind or other slow things like that.  In normal conditions,
the total runtime is not affected when they are run with N=1 as part of
the whole test suite.

> Some issues/inefficiencies regarding this fix and base code related to
> db-specific snapshots built during decoding: [...]

Thanks for spending time reviewing this code.  I think none of these
problems are fundamental in nature, but they are obviously worth
addressing for 19.  If we hit some roadblock, we can still revert only
db-specific snapshots.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Puedes vivir sólo una vez, pero si lo haces bien, una vez es suficiente"