Re: Adding REPACK [concurrently]
Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Hi Antonin,
On Wed, Apr 1, 2026 at 10:36 PM Antonin Houska <ah@cybertec.at> wrote:
> Srinath Reddy Sadipiralla <srinath2133@gmail.com> wrote:
>
> > i was fuzz testing v48 , and found a crash when REPACK (concurrently)
> itself errors out,
> > 1) while running
> >
> > create table test(id int);
> > REPACK (concurrently) test;
> >
> > TBH i didn't knew this, sometimes it's better to not know "rules" ;)
> > [NOTE: maybe we should add that we can't run
> > REPACK (concurrently) on table without identity index or primary key
> into repack.sgml]
> >
> > ERROR: cannot process relation "test"
> > 2026-04-01 19:06:31.211 IST [2495575] HINT: Relation "test" has no
> identity index.
> > 2026-04-01 19:06:31.211 IST [2495575] STATEMENT: repack (concurrently)
> test;
> > TRAP: failed Assert("proc->statusFlags ==
> ProcGlobal->statusFlags[proc->pgxactoff]"), File: "procarray.c", Line: 719,
> PID: 2495575
> > Here's the diff to solve this crash.
>
> Thanks. Attached here is v48-0006 fixed.
>
On Wed, Apr 1, 2026 at 8:25 PM Srinath Reddy Sadipiralla <
srinath2133@gmail.com> wrote:
> Here's the diff to solve this crash.
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
> index 29ba49744eb..d44092a407a 100644
> --- a/src/backend/commands/repack.c
> +++ b/src/backend/commands/repack.c
> @@ -284,7 +284,23 @@ ExecRepack(ParseState *pstate, RepackStmt *stmt, bool
> isTopLevel)
> * that others can conflict with.
> */
> if ((params.options & CLUOPT_CONCURRENT) != 0)
> + {
> + /*
> + * Do not let other backends wait for our completion during their
> + * setup of logical replication. Unlike logical replication publisher,
> + * we will have XID assigned, so the other backends - whether
> + * walsenders involved in logical replication or regular backends
> + * executing also REPACK (CONCURRENTLY) - would have to wait for our
> + * completion before they can build their initial snapshot. It is o.k.
+ * for any decoding backend to ignore us because we do not change
> + * tuple descriptor of any table, and the data changes we write should
> + * not be decoded by other backends.
> + */
> + LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
> MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
> + ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
> + LWLockRelease(ProcArrayLock);
> + }
>
> /*
> * If a single relation is specified, process it and we're done ... unless
> @@ -988,22 +1004,6 @@ rebuild_relation(Relation OldHeap, Relation index,
> bool verbose,
>
> if (concurrent)
> {
> - /*
> - * Do not let other backends wait for our completion during their
> - * setup of logical replication. Unlike logical replication publisher,
> - * we will have XID assigned, so the other backends - whether
> - * walsenders involved in logical replication or regular backends
> - * executing also REPACK (CONCURRENTLY) - would have to wait for our
> - * completion before they can build their initial snapshot. It is o.k.
> - * for any decoding backend to ignore us because we do not change
> - * tuple descriptor of any table, and the data changes we write should
> - * not be decoded by other backends.
> - */
> - LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
> - MyProc->statusFlags |= PROC_IN_CONCURRENT_REPACK;
> - ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
> - LWLockRelease(ProcArrayLock);
> -
> /*
> * The worker needs to be member of the locking group we're the leader
> * of. We ought to become the leader before the worker starts. The
i think as i did earlier in the diff, shouldn't we remove the duplicate code
from rebuild_relation, am i missing something?
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Allow old WAL recycling during REPACK CONCURRENTLY
- 45b02984e2fa 19 (unreleased) landed
-
Advance restart_lsn more eagerly in LogicalConfirmReceivedLocation
- 38470c2c1ea7 19 (unreleased) landed
-
Remove unnecessary signal handler change
- 5d48d3b14e0e 19 (unreleased) landed
-
Improve REPACK (CONCURRENTLY) error messages some more
- 378dffaf8c80 19 (unreleased) landed
-
Revert "Allow logical replication snapshots to be database-specific"
- 01a80f062146 19 (unreleased) landed
-
Move REPACK (CONCURRENTLY) test out of stock regression tests
- 4b2aa4b39cba 19 (unreleased) landed
-
REPACK: do not require REPLICATION or LOGIN
- 5dbb63fc82b7 19 (unreleased) landed
-
Add missing initialization
- 05c401d5786a 19 (unreleased) landed
-
Simplify declaration of memcpy target
- 2cff363715ef 19 (unreleased) landed
-
Reserve replication slots specifically for REPACK
- e76d8c749c31 19 (unreleased) landed
-
doc: Add an example of REPACK (CONCURRENTLY)
- 8fb95a8ab6e5 19 (unreleased) landed
-
Allow logical replication snapshots to be database-specific
- 0d3dba38c777 19 (unreleased) landed
-
Avoid different-size pointer-to-integer cast
- a3b069ef90bd 19 (unreleased) landed
-
Fix valgrind failure
- 5bcc3fbd196c 19 (unreleased) landed
-
Add CONCURRENTLY option to REPACK
- 28d534e2ae0a 19 (unreleased) landed
-
Rename cluster.c to repack.c (and corresponding .h)
- c0b53ec06309 19 (unreleased) landed
-
Allow index_create to suppress index_build progress reporting
- caec9d9fadf1 19 (unreleased) landed
-
Make index_concurrently_create_copy more general
- 33bf7318f94c 19 (unreleased) landed
-
Document the 'command' column of pg_stat_progress_repack
- a630ac5c2016 19 (unreleased) landed
-
Introduce the REPACK command
- ac58465e0618 19 (unreleased) landed
-
Toggle logical decoding dynamically based on logical slot presence.
- 67c20979ce72 19 (unreleased) cited
-
Split vacuumdb to create vacuuming.c/h
- c4067383cb2c 19 (unreleased) landed
-
Remove ReorderBufferTupleBuf structure.
- 08e6344fd642 17.0 cited
-
Revert changes to CONCURRENTLY that "sped up" Xmin advance
- 042b584c7f7d 14.4 cited
-
VACUUM: ignore indexing operations with CONCURRENTLY
- d9d076222f5b 14.0 cited