Re: why there is not VACUUM FULL CONCURRENTLY?

From: Antonin Houska <ah(at)cybertec(dot)at>
To: Kirill Reshke <reshkekirill(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: why there is not VACUUM FULL CONCURRENTLY?
Date: 2024-08-27 12:00:50
Message-ID: 76278.1724760050@antos
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)gmail(dot)com> wrote:

> On Fri, 2 Aug 2024 at 11:09, Antonin Houska <ah(at)cybertec(dot)at> wrote:
> >
> > Kirill Reshke <reshkekirill(at)gmail(dot)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

Attachment Content-Type Size
v02-0001-Adjust-signature-of-cluster_rel-and-its-subroutines.patch text/x-diff 14.9 KB
v02-0002-Move-progress-related-fields-from-PgBackendStatus-to.patch text/x-diff 6.4 KB
v02-0003-Move-conversion-of-a-historic-to-MVCC-snapshot-to-a-.patch text/x-diff 5.4 KB
v02-0004-Add-CONCURRENTLY-option-to-both-VACUUM-FULL-and-CLUS.patch text/plain 170.4 KB
v02-0005-Preserve-visibility-information-of-the-concurrent-da.patch text/x-diff 39.0 KB
v02-0006-Add-regression-tests.patch text/x-diff 10.4 KB
v02-0007-Introduce-cluster_max_xlock_time-configuration-varia.patch text/x-diff 20.5 KB
v02-0008-Call-logical_rewrite_heap_tuple-when-applying-concur.patch text/x-diff 26.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Anthonin Bonnefoy 2024-08-27 12:07:02 Re: Segfault in jit tuple deforming on arm64 due to LLVM issue
Previous Message Jim Jones 2024-08-27 11:57:24 Re: [PATCH] Add CANONICAL option to xmlserialize