Re: Adding REPACK [concurrently]

Antonin Houska <ah@cybertec.at>

From: Antonin Houska <ah@cybertec.at>

To: Mihail Nikalayeu <mihailnikalayeu@gmail.com>

Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Fujii Masao <masao.fujii@gmail.com>, Robert Treat <rob@xzilla.net>, Pg Hackers <pgsql-hackers@lists.postgresql.org>

Date: 2025-08-25T17:22:14Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Improve REPACK (CONCURRENTLY) error messages some more
- 378dffaf8c80 19 (unreleased) landed
Add CONCURRENTLY option to REPACK
- 28d534e2ae0a 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
Split vacuumdb to create vacuuming.c/h
- c4067383cb2c 19 (unreleased) landed
Revert changes to CONCURRENTLY that "sped up" Xmin advance
- 042b584c7f7d 14.4 cited

Mihail Nikalayeu <mihailnikalayeu@gmail.com> wrote:

> Hi, Antonin
> 
> > How does HeapTupleSatisfiesSelf() recognize the status of any XID w/o using a
> > snapshot? Do you mean by checking the commit log (TransactionIdDidCommit) ?
> 
> Yes, TransactionIdDidCommit.

I think the problem is that HeapTupleSatisfiesSelf() uses
TransactionIdIsInProgress() instead of checking the snapshot:

        ...
        else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
                return false;
        else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
        ...

When decoding (and replaying) data changes, you deal with the database state
as it was (far) in the past. However TransactionIdIsInProgress() is not
suitable for this purpose.

And since CommitTransaction() updates the commit log before removing the
transaction from ProcArray, I can even imagine race conditions: if a
transaction is committed and decoded fast enough, TransactionIdIsInProgress()
might still return true. In such a case, HeapTupleSatisfiesSelf() returns
false instead of calling TransactionIdDidCommit().

> Another option is just invent a new
> snapshot type - SnapshotBelieveEverythingCommitted - for that
> particular case it should work - because all xmin/xmax written into
> the new table are committed by design.

I'd prefer optimization of the logical decoding for REPACK CONCURRENTLY, and
using the MVCC snapshots.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com