MIME-Version: 1.0
References: <202507262156.sb455angijk6@alvherre.pgsql>
In-Reply-To: <202507262156.sb455angijk6@alvherre.pgsql>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Sun, 27 Jul 2025 15:00:51 +0900
Message-ID: 
 <CAHGQGwELP6OAOprxjkS5iYSj1KXKmVPULJyYMddE7-uRxrSP_Q@mail.gmail.com>
Subject: Re: Adding REPACK [concurrently]
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>,
 Antonin Houska <ah@cybertec.at>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAHGQGwELP6OAOprxjkS5iYSj1KXKmVPULJyYMddE7-uRxrSP_Q%40mail.gmail.com>
Precedence: bulk

On Sun, Jul 27, 2025 at 6:56=E2=80=AFAM Alvaro Herrera <alvherre@alvh.no-ip=
.org> wrote:
>
> Hello,
>
> Here's a patch to add REPACK and eventually the CONCURRENTLY flag to it.
> This is coming from [1].  The ultimate goal is to have an in-core tool
> to allow concurrent table rewrite to get rid of bloat;

+1


> right now, VACUUM
> FULL does that, but it's not concurrent.  Users have resorted to using
> the pg_repack third-party tool, which is ancient and uses a weird
> internal implementation, as well as pg_squeeze, which uses logical
> decoding to capture changes that occur during the table rewrite.  The
> patch submitted here, largely by Antonin Houska with some changes by me,
> is based on the the pg_squeeze code which he authored, and first
> introduces a new command called REPACK to absorb both VACUUM FULL and
> CLUSTER, followed by addition of a CONCURRENTLY flag to allow some forms
> of REPACK to operate online using logical decoding.

Does this mean REPACK CONCURRENTLY requires wal_level =3D logical,
while plain REPACK (without CONCURRENTLY) works with any wal_level
setting? If we eventually deprecate VACUUM FULL and CLUSTER,
I think plain REPACK should still be allowed with wal_level =3D minimal
or replica, so users with those settings can perform equivalent
processing.


+ if (!cluster_is_permitted_for_relation(tableOid, userid,
+    CLUSTER_COMMAND_CLUSTER))

As for the patch you attached, it seems to be an early WIP and
might not be ready for review yet?? BTW, I got the following
compilation failure and probably CLUSTER_COMMAND_CLUSTER
the above should be GetUserId().

-----------------
cluster.c:455:14: error: use of undeclared identifier 'CLUSTER_COMMAND_CLUS=
TER'
  455 |
                    CLUSTER_COMMAND_CLUSTER))
      |
                    ^
1 error generated.
-----------------

Regards,

--=20
Fujii Masao