Re: why there is not VACUUM FULL CONCURRENTLY?

Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Mar 20, 2025 at 2:09 PM Antonin Houska <ah(at)cybertec(dot)at> wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > > Is there a README or a long comment in here someplace that is a good
> > > place to read to understand the overall design of this feature?
> >
> > I tried to explain how it works in the commit messages. The one in 0004 is
> > probably the most important one.
>
> Thanks. A couple of comments/questions:
>
> - I don't understand why this commit message seems to think that we
> can't acquire a stronger lock while already holding a weaker one. We
> can do that, and in some cases we do precisely that.

Can you please give me an example? I don't recall seeing a lock upgrade in the
tree. That's the reason I tried rather hard to avoid that.

> Such locking
> patterns can result in deadlock e.g. if I take AccessShareLock and you
> take AccessShareLock and then I tried to upgrade to
> AccessExclusiveLock and then you try to upgrade to
> AccessExclusiveLock, somebody is going to have to ERROR out. But that
> doesn't keep us from doing that in some places where it seems better
> than the alternatives, and the alternative chosen by the patch
> (possibly discovering at the very end that all our work has been in
> vain) does not seem better than risking a deadlock.

I see. Only the backends that do upgrade their lock are exposed to the risk of
deadlock, e.g. two backends running REPACK CONCURRENTLY on the same table, and
that should not happen too often.

I'll consider your objection - it should make the patch a bit simpler.

> - On what basis do you make the statement in the last paragraph that
> the decoding-related lag should not exceed one WAL segment? I guess
> logical decoding probably keeps up pretty well most of the time but
> this seems like a very strong guarantee for something I didn't know we
> had any kind of guarantee about.

The patch itself does guarantee that by checking the amount of unprocessed WAL
regularly when it's copying the data into the new table. If too much WAL
appears to be unprocessed, it enforces the decoding before the copying is
resumed.

The WAL decoding during the "initial load" phase can actually be handled by a
background worker (not sure it's necessary in the initial implementation),
which would make a significant lag rather unlikely. But even then we should
probably enforce certain limit on the lag (e.g. because background worker is
not guaranteed to start).

> - What happens if we crash?

The replication slot we create is RS_TEMPORARY, so it disappears after
restart. Everything else is as if the current implementation of CLUSTER ends
due to crash.

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