Re: Adding REPACK [concurrently]
Antonin Houska <ah@cybertec.at>
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > I offer the following rather trivial fixup diffs, which I think should > be mostly be for 0002. Thanks, just a few comments: * 0002 - I didn't add the 'repack_' prefix because the function is static, but realize now that it might be useful from the code browsing perspective. * 0008 - It's possible during query execution, when you know exactly which attributes you need to fetch from the tuple. However in store_change(), we don't know which attributes are external (indirect) without deforming them all. > Other trivial things I'd like to change, if you don't mind, > - the name pgoutput_repack sounds wrong to me. I would rather say > rpck_output, repack_output, repack_plugin, ... or something. I don't > understand where the suffix "output" comes from in the name > "pgoutput", but it sounds like arbitrary nonsense to me. No strong preference here, except that I don't like "rpck_...". How about pgoutput/repack.c? I think I tried to put the plugin into the existing "pgoutput" directory at some point, but don't remember why I eventually rejected that approach. > - I would rename the routines in that file to also have the name > "repack", probably as prefixes. ok > One thing we need and is rather not trivial, is to go through the table > AM interface rather than calling heapam.c routines directly. I'm > working on this now and will present a patch later. It occurred to me too, at least because copy_table_data() calls table_relation_copy_for_cluster() rather than heapam_relation_copy_for_cluster(). Thanks for working on it. > Another thing I noticed while going over the code was that we seem to > spill whole tuples even for things like the old tuple of an UPDATE and > for DELETE, but unless I misunderstand how this works, these shouldn't > be necessary: we just need the replica identity so that we can locate > the tuple to operate on. Especially for tuples that contain large > toasted attributes, this is likely important. I think we don't need to care, as both heap_update() and heap_delete() call ExtractReplicaIdentity(). That returns a real tuple, but it only contains the attributes constituting the replica identity. The other ones are set to NULL. > It may make sense to use the TupleTableSlot interface rather than > HeapTuple for everything. I'm not really sure about this though. Isn't this part of the adoption of table AM? For example, table_tuple_insert() accepts tuple slot rather than heap tuple. -- 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