Feedback-ID: ia2694551:Fastmail
Date: Tue, 9 Dec 2025 20:22:09 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Antonin Houska <ah@cybertec.at>
Cc: Mihail Nikalayeu <mihailnikalayeu@gmail.com>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>,
 Robert Treat <rob@xzilla.net>
Subject: Re: Adding REPACK [concurrently]
Message-ID: <202512091915.ws6fk7csn76r@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <171530.1765306357@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/202512091915.ws6fk7csn76r%40alvherre.pgsql>
Precedence: bulk

Hello, many thanks for the new version.  Here's a very quick proposal
for a new top-of-file comment on cluster.c,

 * cluster.c
 *		Implementation of REPACK [CONCURRENTLY], also known as CLUSTER and
 *		VACUUM FULL.
 *
 * There are two somewhat different ways to rewrite a table.  In non-
 * concurrent mode, it's easy: take AccessExclusiveLock, create a new
 * transient relation, copy the tuples over to the relfilenode of the
 * new relation, swap the relfilenodes, then drop the old relation.
 *
 * In concurrent mode, we lock the table with only ShareUpdateExclusiveLock,
 * then do an initial copy as above.  However, while the tuples are being
 * copied, concurrent transactions could modify the table, and to cope
 * with those changes, we rely on logical decoding to obtain them from WAL.
 * A bgworker consumes WAL while the initial copy is ongoing (to prevent
 * excessive WAL from being reserved), and accumulates the changes in
 * a tuplestore.  Once the initial copy is complete, we read the changes
 * from the tuplestore and re-apply them on the new heap.  Then we
 * upgrade our ShareUpdateExclusiveLock to AccessExclusiveLock and swap
 * the relfilenodes.  This way, the time we hold a strong lock on the
 * table is much reduced, and the bloat is greatly reduced.

I haven't read build_relation_finish_concurrent() yet to understand how
exactly do we do the lock upgrade, which I think is an important point
we should address in this comment.  Also not addressed is how exactly we
handle indexes.  Feel free to correct this, reword it or include any
additional details that you think are important.


(At this point we could just as well rename the file to repack.c, since
very little of the original remains.  But let's discuss that later.)

Thanks,

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Doing what he did amounts to sticking his fingers under the hood of the
implementation; if he gets his fingers burnt, it's his problem."  (Tom Lane)