Re: why there is not VACUUM FULL CONCURRENTLY?

Antonin Houska <ah@cybertec.at>

From: Antonin Houska <ah@cybertec.at>
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Pavel Stehule <pavel.stehule@gmail.com>, Michael Paquier <michael@paquier.xyz>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-08-27T12:00:50Z
Lists: pgsql-hackers

Attachments

Attached is version 2, the feature itself is now in 0004.

Unlike version 1, it contains some regression tests (0006) and a new GUC to
control how long the AccessExclusiveLock may be held (0007).

Kirill Reshke <reshkekirill@gmail.com> wrote:

> On Fri, 2 Aug 2024 at 11:09, Antonin Houska <ah@cybertec.at> wrote:
> >
> > Kirill Reshke <reshkekirill@gmail.com> wrote:
> > > However, in general, the 3rd patch is really big, very hard to
> > > comprehend.  Please consider splitting this into smaller (and
> > > reviewable) pieces.
> >
> > I'll try to move some preparation steps into separate diffs, but not sure if
> > that will make the main diff much smaller. I prefer self-contained patches, as
> > also explained in [3].
> 
> Thanks for sharing [3], it is a useful link.
> 
> There is actually one more case when ACCESS EXCLUSIVE is held: during
> table rewrite (AT set TAM, AT set Tablespace and AT alter column type
> are some examples).
> This can be done CONCURRENTLY too, using the same logical replication
> approach, or do I miss something?

Yes, the logical replication can potentially be used in other cases.

> I'm not saying we must do it immediately, this should be a separate
> thread, but we can do some preparation work here.
> 
> I can see that a bunch of functions which are currently placed in
> cluster.c can be moved to something like
> logical_rewrite_heap.c. ConcurrentChange struct and
> apply_concurrent_insert function  is one example of such.
> 
> So, if this is the case, 0003 patch can be splitted in two:
> The first one is general utility code for logical table rewrite
> The second one with actual VACUUM CONCURRENTLY feature.

> What do you think?

I can imagine moving the function process_concurrent_changes() and subroutines
to a different file (e.g. rewriteheap.c), but moving it into a separate diff
that does not contain any call of the function makes little sense to me. Such
a diff would not add any useful functionality and could not be considered
refactoring either.

So far I at least moved some code to separate diffs: 0003 and 0005. I'll move
more if I find sensible opportunity in the future.

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

Commits

  1. Add CONCURRENTLY option to REPACK

  2. Introduce the REPACK command

  3. Adjust signature of cluster_rel() and its subroutines

  4. Simplify signature of RewriteTable