Re: Adding REPACK [concurrently]
Antonin Houska <ah@cybertec.at>
Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote: > When testing REPACK concurrently, I noticed that all WALs are retained from > the moment REPACK begins copying data to the new table until the command > finishes replaying concurrent changes on the new table and stops the repack > decoding worker. > > I understand the reason: the REPACK command itself starts a long-running > transaction, and logical decoding does not advance restart_lsn beyond the > oldest running transaction's start position. As a result, slot.restart_lsn > remains unchanged, preventing the checkpointer from recycling WALs. I think you're right, sorry for the omission. > However, since REPACK can run for a long time (hours or even days), I'd like > to confirm whether this is expected behavior or if we plan to improve it > in the future ? And additionally, Yes, it will be improved. I have a draft patch for it, will rebase and post it soon. The plan is to: 1) preserve the original xmin/xmax of the tuples when we insert them into the new heap. Thus, besides achieving MVCC safety, we won't need XID assigned for most of the time. 2) do catalog changes in separate transactions - XID needed here, but these transactions take very short time. 3) use a single snapshot only for limited number of tuples/pages. When more data needs to be copied, a new snapshot is built, supposedly with higher ->xmin than the prevous one. > IIUC, REPACK without using concurrent option does not have this issue. It does not have the WAL recycling issue because it does not need to read WAL. However it also runs in a long transaction. Even though it does not need XID for the actual heap rewriting, it gets one at the moment it locks the table using AccessExclusiveLock (which is at the very beginning). > Given that we do not restart a REPACK, I think the repack decoding worker > should be able to advance restart_lsn each time after writing changes > (similar to how a physical slot behaves). To illustrate this, I've written > a patch (attached) that implements this approach, and it works fine for me. LGTM, thanks! > BTW, catalog_xmin also won't advance, but that seems not a big issue as > the REPACK transaction itself also holds a snapshot that retains catalog tuples, > so advancing catalog_xmin wouldn't change the situation anyway. The snapshot "resetting" (mentioned above) should fix this problem too. -- Antonin Houska Web: https://www.cybertec-postgresql.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