Re: Adding REPACK [concurrently]

On Wed, May 27, 2026 at 10:18 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Thursday, May 28, 2026 11:34 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > On Wed, May 27, 2026 at 5:31 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> > wrote:
> > >
> > > On Wed, May 27, 2026 at 1:08 AM Zhijie Hou (Fujitsu)
> > > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > > >
> > > > 0001 remains unchanged.
> > > >
> > >
> > > Few minor comments:
> > > =================
> >
> > Commit message says: "This change does not advance catalog_xmin.
> > REPACK already holds a snapshot that prevents catalog dead tuple removal,
> > so catalog_xmin handling can be addressed independently.".
> > Isn't it equally important to advance this, otherwise, for long running REPACKs
> > dead tuples will be accumulated needlessly? If so, do we have any ideas to
> > avoid this?
>
> My understanding is that dead tuple accumulation is common to all long-running
> commands (including CLUSTER, VACUUM FULL, and REPACK without CONCURRENTLY). As
> long as a command holds a snapshot for a long time while scanning and copying
> data, the backend xmin will cause similar accumulation. So, this doesn't seem
> like a new issue to me, and given that catalog_xmin only affect tuples in system
> catalog which is less harmful, I thought it could be handled independently.
> There was a proposal to improve this case in [1].
>

Fair enough. It makes sense to deal with catalog_xmin separately.

> Attaching the v4 patch which improved the comments and commit message as
> suggested.
>

I haven't tested it but otherwise the code changes looks good to me.

--
With Regards,
Amit Kapila.