Horton
Menu

Re: Adding REPACK [concurrently]

Mihail Nikalayeu <mihailnikalayeu@gmail.com>

View thread
From: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
To: Antonin Houska <ah@cybertec.at>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Pg Hackers <pgsql-hackers@lists.postgresql.org>, Robert Treat <rob@xzilla.net>
Date: 2026-01-22T11:30:00Z
Lists: pgsql-hackers 
Hello, Antonin!

> The changes present in WAL decoded prior the snapshot creation are not
> replayed - these changes are visible to the snapshot. (This is not really
> specific to the 0006 part.)

OK, just want to be sure it still works the same way if we build multiple
snapshots for the same slot that way.

> The current API does not seem to support changing snapshot of an
in-progress
> scan and I don't want to change that. Plus note that the current
> implementation of CLUSTER also uses SnapshotAny and then checks the
visibility
> separately. Finally, SnapshotAny is not really an expensive visibility
check,
> if it can be considered a visibility check at all.

But we will require a real check for each tuple. Including dead one,
multiple versions of the same HOT, etc.

> I've added it only for xmin. xid is valid because REPACK is executed in a
> transaction. That reminds me that PROC_IN_VACUUM should be present in
> MyProc->statusFlags. Fixed.

Yes, xid is required for repack. I think it is better to introduce a new
flag instead of PROC_IN_VACCUUM.


> > > PushActiveSnapshot(GetTransactionSnapshot());
> > GetLatestSnapshot() feels better here.
> What will then happen to code that uses GetActiveSnapshot() ?

O, I mean PushActiveSnapshot(GetLatestSnapshot())

> > Also, to correctly build a unique index - some tech from [0] is
required (building a unique index with multiple snapshots is a little bit
tricky).
> ok, I'll check your patch.

I realized building a unique index is still done with a single snapshot, so
it should be OK for that case. But still check the patch :)

>  I proposed the Assert above, but still thinking about it.
Hm... Do we really need these asserts if PROC_IN_VACUUM is set? I was
proposing a way it is used for index building (to ensure nothing is
propagated into xmin).

Best regards,
Mikhail.

Commits

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

  1. Allow old WAL recycling during REPACK CONCURRENTLY

  2. Advance restart_lsn more eagerly in LogicalConfirmReceivedLocation

  3. Remove unnecessary signal handler change

  4. Improve REPACK (CONCURRENTLY) error messages some more

  5. Revert "Allow logical replication snapshots to be database-specific"

  6. Move REPACK (CONCURRENTLY) test out of stock regression tests

  7. REPACK: do not require REPLICATION or LOGIN

  8. Add missing initialization

  9. Simplify declaration of memcpy target

  10. Reserve replication slots specifically for REPACK

  11. doc: Add an example of REPACK (CONCURRENTLY)

  12. Allow logical replication snapshots to be database-specific

  13. Avoid different-size pointer-to-integer cast

  14. Fix valgrind failure

  15. Add CONCURRENTLY option to REPACK

  16. Rename cluster.c to repack.c (and corresponding .h)

  17. Allow index_create to suppress index_build progress reporting

  18. Make index_concurrently_create_copy more general

  19. Document the 'command' column of pg_stat_progress_repack

  20. Introduce the REPACK command

  21. Toggle logical decoding dynamically based on logical slot presence.

  22. Split vacuumdb to create vacuuming.c/h

  23. Remove ReorderBufferTupleBuf structure.

  24. Revert changes to CONCURRENTLY that "sped up" Xmin advance

  25. VACUUM: ignore indexing operations with CONCURRENTLY