Fix reindexdb with parallel index-level conrurrent run
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254036psql -h localhost -U postgresBuilt from patchset v6 (message #6), October 06, 2026 at 07:10 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254036_6 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254036_6 && git checkout t254036_6Patchset v6 (message #6) is on t254036_6
Hi!
$subj doesn't work. I bumped into it while doing some corruption fixing.
repro:
create table z(i int);
create index z_1 on z(i);
create index z_2 on z(i);
reshke@pg-bench-dc:~/pgmaster$
/home/reshke/pgmaster/pgbinm/bin/reindexdb -p 7777 --concurrently -i
z_1 -i z_2 -j 1
reshke@pg-bench-dc:~/pgmaster$
/home/reshke/pgmaster/pgbinm/bin/reindexdb -p 7777 --concurrently -i
z_1 -i z_2 -j 2
reindexdb: error: processing of database "reshke" failed: ERROR:
REINDEX CONCURRENTLY cannot run inside a transaction block
The root cause is that reindexdb tries to parallelize REINDEX by
batching all commands related to one table in one job. However,
implementation of this makes multiple DDL query an multistatement
which is transformed into implicit transaction server-side.
Oversight of 47f99a407d. I didn't find any related bug report, so it
looks like this feature is not widely used. affected versions and
17-19 and master.
PFA fix.
--
Best regards,
Kirill Reshke
Hi Kirill,
reindexdb: error: processing of database "reshke" failed: ERROR:
REINDEX CONCURRENTLY cannot run inside a transaction block
Reproduced on master, REL_18 and REL_17. With the patch, every
--concurrently --jobs case I tried rebuilds all the requested
indexes, and the new test in 090_reindexdb.pl fails without the
reindexdb.c change and passes with it.
I measured it as well, and a few things came up, all in the new
branch.
1. --jobs no longer runs in parallel. The branch waits for every
command with consumeQueryResult(), even for an index that is the only
one of its table, and the main loop hands out nothing while it waits.
Four tables with one index each, 3M rows per table, median of 6
rounds, same server for all clients:
reindexdb --concurrently -j 4 -i t1_b -i t2_b -i t3_b -i t4_b
master: 6.3 s
v1: 22.5 s
v2 (attached): 6.6 s
(-j 1: 22.5 s)
2. A cancel only stops the current index. The inner loop never looks
at CancelRequested, so after Ctrl-C reindexdb goes on with the rest
of the table's indexes. With one table of three 2M-row indexes and
SIGINT after 3 s, -j 1 exits at once with nothing more rebuilt, while
v1 with -j 2 exits 9 s later, having rebuilt the other two. For the
same reason a failed REINDEX does not stop the run, while elsewhere
reindexdb stops at the first failure (with -j 1, and with -j 2
without --concurrently).
3. Back-patching. ParallelSlotSetIdle() only exists from REL_19 on
(750816971b3), so the patch does not compile on REL_18 or REL_17;
"free_slot->inUse = false" works there. On REL_17 there is a quieter
trap: indices_tables_list is a SimpleStringList, and the existing
branch compares it with strcmp(). The new "==" compiles, but it
compares pointers and never matches. With v1 alone that goes
unnoticed, because everything runs one at a time anyway, but once a
single-index table may run asynchronously, as with 0002, the indexes
of one table end up in different jobs: REL_17 then fails with
"deadlock detected" in 5 runs out of 5, and passes 5 out of 5 with
strcmp().
Attached is v2 as two patches:
0001 is your patch, unchanged. It had no commit message, so I wrote
one from your mail; please change it as you like.
0002 addresses 1 and 2. It takes the waiting path only when the next
index belongs to the same table, so a single-index table goes through
the normal asynchronous path, as on master. It also stops at the
first failure or cancel. 090 passes, and the cancel case exits at
once with nothing more rebuilt.
With the two changes from point 3, 0001+0002 and 090 pass on REL_18
and REL_17 (on 17 the test hunk needs the short -i option). I can
post versions for those branches if that helps.
What v2 does not solve: the main loop still waits while a multi-index
table is processed. Tables are ordered by the size of their largest
index, ascending, so a small multi-index table is handed out first
and delays the larger ones. One table with two 1.5M-row indexes plus
three tables with one 3M-row index, -j 4:
v1: 23.6 s
v2: 12.0 s
the two-index table alone: 5.1 s
the other three alone, -j 3: 6.5 s
Fixing that would need the slot to send the table's next command
itself, which is a larger change than I would put in a back-patched
fix.
Regards,
Manu
On Fri, Oct 02, 2026 at 10:03:39PM +0500, Kirill Reshke wrote:
The root cause is that reindexdb tries to parallelize REINDEX by
batching all commands related to one table in one job. However,
implementation of this makes multiple DDL query an multistatement
which is transformed into implicit transaction server-side.Oversight of 47f99a407d. I didn't find any related bug report, so it
looks like this feature is not widely used. affected versions and
17-19 and master.
Sending such commands one-by-one also makes me think that we should
have some smarter parallel queuing, but I don't really know how much
better we could do on top of what's already in the tree. For the
parallel index-level non-concurrent case, we already order the
commands so as the same table is not worked on at the same time across
multiple slots to avoid conflicts.
Duplicating run_reindex_command() to be called in these three
different code branches vs once previously puts the code in a worse
state. On HEAD, we have one or more gen_reindex_command() for each
branch, wrapped by one single run_reindex_command(). Perhaps the
result would be nice if we rethink the gen/run interfaces, just that
the succession of two gen/run_reindex_command calls does not give me a
warm feeling: it makes the reasoning of the gen/run steps harder to
think about.
Btw, my first impression was: is there a huge use case for the support
of this kind of mode anyway? When running under CONCURRENTLY at index
level, parallel jobs have limited interest as the wait phases could
take a long time, so I'd wonder about just returning an error first,
and rethink better the code to support this case. Finally, perhaps
all that is not really worth the effort, especially because
table-level concurrent works (er, right?).
--
Michael
Hi Michael,
so I'd wonder about just returning an error first
Agreed. The only case that fails is two or more of the named indexes
sharing a table with -j >= 2 (they batch into one multi-statement query).
And for that case --jobs cannot parallelize anyway: the indexes of one
table have to be reindexed one at a time to avoid deadlocks. So fixing the
parallel path unlocks no speed, it only removes the error. Erroring out
looks right to me; a clear message would beat today's "cannot run inside a
transaction block". Happy to write that.
Regards,
Manu
On Mon, Oct 05, 2026 at 04:42:21PM -0300, Manu wrote:
Happy to write that.
I'd suggest to let Kirill reply first.
--
Michael
On Tue, 6 Oct 2026 at 08:36, Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Oct 05, 2026 at 04:42:21PM -0300, Manu wrote:
Happy to write that.
I'd suggest to let Kirill reply first.
--
Michael
Hi, thank you for your response.
I think we can indeed just drop support for parallel index-level
concurrency index rebuild. PFA
--
Best regards,
Kirill Reshke
On Tue, Oct 06, 2026 at 11:56:21AM +0500, Kirill Reshke wrote:
I think we can indeed just drop support for parallel index-level
concurrency index rebuild. PFA
WFM. If somebody is interested in this pattern (I suspect that's not
the case, maybe I'm wrong), then it could always happen later.
--
Michael
Hi Kirill,
I think we can indeed just drop support for parallel index-level
concurrency index rebuild.
+1. I tested v1 on master: --concurrently --jobs at index level now
fails cleanly with "cannot use multiple jobs to reindex indexes
concurrently", while --concurrently without --jobs and table-level
--concurrently --jobs are unaffected. Reads right to me.Regards,
Manu
On Tue, Oct 06, 2026 at 04:06:08PM +0900, Michael Paquier wrote:
WFM. If somebody is interested in this pattern (I suspect that's not
the case, maybe I'm wrong), then it could always happen later.
The error message has been moved close to the catalog check after the
options have been parsed instead of being hidden in the internals of
the reindex, and applied as e9c03f0f1bf6 down to v17.
--
Michael