Re: Adding REPACK [concurrently]

Hi,

On 2026-04-09 10:26:22 -0400, Andres Freund wrote:
> On 2026-04-09 16:06:17 +0200, Mihail Nikalayeu wrote:
> > On Thu, Apr 9, 2026 at 11:26 AM Antonin Houska <ah(at)cybertec(dot)at> wrote:
> > > Sure, it's possible, but IMO the principal question is whether REPACK should
> > > let VACUUM and DDLs error out, or just let them wait.
> >
> > One more idea: instead of ERROR in CheckTableNotInUse in case of
> > in_repack - just release the lock and retry a little later (by that
> > time, the repack operation will likely have acquired the AEL).
>
> I continue to think this approach has no chance of working. Even if it
> theoretically could, there are lots of paths to locking relations that do not
> go through CheckTableNotInUse(), and we're not going to just route them all
> through CheckTableNotInUse().
>
> What CheckTableNotInUse() is for is to prevent DDL from changing the structure
> of the table when it is still being referred to. You can't just call
> CheckTableNotInUse() from a LOCK TABLE - it would trigger wrong errors *ALL
> THE TIME* because a LOCK TABLE does not need to error out just because there's
> also a cursor on the table.
>
>
> And it'd trigger lots of bogus errors. See the first example in
> https://postgr.es/m/fpr4nsmyy3mpfrm2mijspr44dgol2cjeke5tyznb4btsznxsgx%40iifdbfe2wl63
>
> S2 would get the lock and then error out due to the proposed check. Even
> though there's no need for it.

Before you protest that you could just let the non-DDL operation have its
lock, sure, but that's an utterly terrible idea, because it will lead to more
and more work accumulating that then has to happen when the AEL is actually
held.

Greetings,

Andres Freund