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
From c52841347f12acc351a2b896b1b91054e7edae90 Mon Sep 17 00:00:00 2001 From: Kirill Reshke <[email protected]> Date: Fri, 2 Oct 2026 22:45:09 -0300 Subject: [PATCH v2 1/2] Fix reindexdb --jobs --concurrently with indexes of the same table For parallel index-level REINDEX, reindexdb puts the commands for all the indexes of one table into a single query. A multi-statement simple query runs as an implicit transaction block, where REINDEX CONCURRENTLY is not allowed, so the run failed. With --concurrently, send each command separately instead. Oversight in 47f99a407d. --- src/bin/scripts/reindexdb.c | 43 ++++++++++++++++++++++++++-- src/bin/scripts/t/090_reindexdb.pl | 24 ++++++++++++++++ src/fe_utils/parallel_slot.c | 2 +- src/include/fe_utils/parallel_slot.h | 9 ++++++ 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index d7fb16d3c85..d7784de3f55 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -434,7 +434,7 @@ reindex_one_database(ConnParams *cparams, ReindexType type, ParallelSlotSetHandler(free_slot, TableCommandResultHandler, NULL); initPQExpBuffer(&sql); - if (parallel && process_type == REINDEX_INDEX) + if (parallel && process_type == REINDEX_INDEX && !concurrently) { /* * For parallel index-level REINDEX, the indices of the same table @@ -455,14 +455,51 @@ reindex_one_database(ConnParams *cparams, ReindexType type, echo, verbose, concurrently, tablespace, &sql); } indices_tables_cell = indices_tables_cell->next; + run_reindex_command(free_slot->connection, process_type, objname, + echo, &sql); + } + else if (parallel && process_type == REINDEX_INDEX) + { + /* + * REINDEX CONCURRENTLY cannot run in a transaction block, so it + * cannot be part of a multi-statement simple query, which would + * be an implicit transaction block. The indices of the same + * table still have to be processed by the same job, to avoid + * concurrent REINDEX CONCURRENTLY commands on the same table, + * which could deadlock. So, each command is sent separately, + * waiting for it to complete before sending the next one. + */ + gen_reindex_command(free_slot->connection, process_type, objname, + echo, verbose, concurrently, tablespace, &sql); + run_reindex_command(free_slot->connection, process_type, objname, + echo, &sql); + if (!consumeQueryResult(free_slot)) + failed = true; + while (indices_tables_cell->next && + indices_tables_cell->val == indices_tables_cell->next->val) + { + indices_tables_cell = indices_tables_cell->next; + cell = cell->next; + objname = cell->val; + termPQExpBuffer(&sql); + initPQExpBuffer(&sql); + gen_reindex_command(free_slot->connection, process_type, objname, + echo, verbose, concurrently, tablespace, &sql); + run_reindex_command(free_slot->connection, process_type, objname, + echo, &sql); + if (!consumeQueryResult(free_slot)) + failed = true; + } + indices_tables_cell = indices_tables_cell->next; + ParallelSlotSetIdle(free_slot); } else { gen_reindex_command(free_slot->connection, process_type, objname, echo, verbose, concurrently, tablespace, &sql); + run_reindex_command(free_slot->connection, process_type, objname, + echo, &sql); } - run_reindex_command(free_slot->connection, process_type, objname, - echo, &sql); termPQExpBuffer(&sql); cell = cell->next; diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl index ae7d3724464..1013120f312 100644 --- a/src/bin/scripts/t/090_reindexdb.pl +++ b/src/bin/scripts/t/090_reindexdb.pl @@ -168,6 +168,30 @@ $node->issues_sql_like( [ 'reindexdb', '--concurrently', '--index' => 'test1x', 'postgres' ], qr/statement: REINDEX INDEX CONCURRENTLY public\.test1x;/, 'reindex specific index concurrently'); + +# Multiple indexes of the same table with parallel jobs must not be +# batched into a single multi-statement query when using --concurrently, +# as REINDEX CONCURRENTLY cannot run inside a transaction block. +$node->safe_psql('postgres', + 'CREATE TABLE test2 (a int); CREATE INDEX test2x ON test2 (a);' + . ' CREATE INDEX test2y ON test2 (a);'); +$node->command_ok( + [ + 'reindexdb', '--jobs' => '2', '--concurrently', + '--index' => 'test2x', + '--index' => 'test2y', + 'postgres', + ], + 'reindex two indexes of the same table concurrently with two jobs'); +$node->command_ok( + [ + 'reindexdb', '--jobs' => '2', + '--index' => 'test2x', + '--index' => 'test2y', + 'postgres', + ], + 'reindex two indexes of the same table with two jobs'); + $node->issues_sql_like( [ 'reindexdb', '--concurrently', '--schema' => 'public', 'postgres' ], qr/statement: REINDEX SCHEMA CONCURRENTLY public;/, diff --git a/src/fe_utils/parallel_slot.c b/src/fe_utils/parallel_slot.c index fb9e6cc4ec1..831e0c27878 100644 --- a/src/fe_utils/parallel_slot.c +++ b/src/fe_utils/parallel_slot.c @@ -54,7 +54,7 @@ processQueryResult(ParallelSlot *slot, PGresult *result) * nothing remains. If at least one error is encountered, return false. * Note that this will block if the connection is busy. */ -static bool +bool consumeQueryResult(ParallelSlot *slot) { bool ok = true; diff --git a/src/include/fe_utils/parallel_slot.h b/src/include/fe_utils/parallel_slot.h index a6ebe273ce0..6d5972614bf 100644 --- a/src/include/fe_utils/parallel_slot.h +++ b/src/include/fe_utils/parallel_slot.h @@ -78,6 +78,15 @@ extern void ParallelSlotsTerminate(ParallelSlotArray *sa); extern bool ParallelSlotsWaitCompletion(ParallelSlotArray *sa); +/* + * Wait for the results of the query currently being processed by the given + * slot, and process them with the handler set for this slot. Returns + * false if the query failed, true otherwise. The slot is not marked as + * idle by this function. Note that this will block if the connection is + * busy. + */ +extern bool consumeQueryResult(ParallelSlot *slot); + extern bool TableCommandResultHandler(PGresult *res, PGconn *conn, void *context); -- 2.55.0
From 7420f3194939e08219634ff079a72671766698ad Mon Sep 17 00:00:00 2001 From: Manuel Reyes Bravo <[email protected]> Date: Fri, 2 Oct 2026 22:45:09 -0300 Subject: [PATCH v2 2/2] reindexdb: keep --jobs parallel and stop on cancel with --concurrently Only wait for each command when the next index belongs to the same table. An index that is the only one of its table goes through the usual asynchronous path, so indexes of different tables are again processed in parallel. Also stop at the first failure or cancel request, as the other paths do, instead of going on with the remaining indexes of the table. --- src/bin/scripts/reindexdb.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index d7784de3f55..4002616cd8f 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -458,7 +458,9 @@ reindex_one_database(ConnParams *cparams, ReindexType type, run_reindex_command(free_slot->connection, process_type, objname, echo, &sql); } - else if (parallel && process_type == REINDEX_INDEX) + else if (parallel && process_type == REINDEX_INDEX && + indices_tables_cell->next && + indices_tables_cell->val == indices_tables_cell->next->val) { /* * REINDEX CONCURRENTLY cannot run in a transaction block, so it @@ -469,32 +471,37 @@ reindex_one_database(ConnParams *cparams, ReindexType type, * which could deadlock. So, each command is sent separately, * waiting for it to complete before sending the next one. */ - gen_reindex_command(free_slot->connection, process_type, objname, - echo, verbose, concurrently, tablespace, &sql); - run_reindex_command(free_slot->connection, process_type, objname, - echo, &sql); - if (!consumeQueryResult(free_slot)) - failed = true; - while (indices_tables_cell->next && - indices_tables_cell->val == indices_tables_cell->next->val) + for (;;) { - indices_tables_cell = indices_tables_cell->next; - cell = cell->next; - objname = cell->val; - termPQExpBuffer(&sql); - initPQExpBuffer(&sql); gen_reindex_command(free_slot->connection, process_type, objname, echo, verbose, concurrently, tablespace, &sql); run_reindex_command(free_slot->connection, process_type, objname, echo, &sql); - if (!consumeQueryResult(free_slot)) + + /* Stop at the first failure or cancel, as the other paths do. */ + if (!consumeQueryResult(free_slot) || CancelRequested) + { + termPQExpBuffer(&sql); failed = true; + goto finish; + } + + if (!(indices_tables_cell->next && + indices_tables_cell->val == indices_tables_cell->next->val)) + break; + indices_tables_cell = indices_tables_cell->next; + cell = cell->next; + objname = cell->val; + resetPQExpBuffer(&sql); } indices_tables_cell = indices_tables_cell->next; ParallelSlotSetIdle(free_slot); } else { + /* The only index of its table: nothing to keep in order. */ + if (parallel && process_type == REINDEX_INDEX) + indices_tables_cell = indices_tables_cell->next; gen_reindex_command(free_slot->connection, process_type, objname, echo, verbose, concurrently, tablespace, &sql); run_reindex_command(free_slot->connection, process_type, objname, -- 2.55.0
