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 |
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 |