Re: REPACK (CONCURRENTLY) decoding worker is canceled by lock_timeout

> but there's no practical effect here, right? If it doesn't die because
> of this particular timeout, it will fail due to some other timeout.

It's not really about two concurrent REPACKs. The worker waits for
every running XID, so a single ordinary write transaction does it:

ALTER DATABASE postgres SET lock_timeout = '2s';
-- session 1: BEGIN; INSERT INTO other VALUES (1); (left open)
-- session 2: SET lock_timeout = 0; REPACK (CONCURRENTLY) t;

ERROR: canceling statement due to lock timeout
CONTEXT: waiting for ShareLock on transaction 2096
REPACK decoding worker

> would statement_timeout also need to be turned off?

It's in the patch already. My first email was sloppy about this. What I
meant was that a timeout or a cancel on the caller's own session still
kills the command, and the patch leaves that alone. statement_timeout
and idle_in_transaction_session_timeout aren't fire in a bgworker
anyway. I only kept them so the block reads the same as autovacuum.

What bothers me is that the caller can't work around it. A session
level SET doesn't reach the worker. ALTER SYSTEM ranks below ALTER
DATABASE, so that doesn't work either. That leaves two options. You can
reset it on the database, but then every session in that database loses
lock_timeout, and it only takes effect for new connections. Or you can
set it on the owner role, but that needs CREATEROLE plus ADMIN on that
role. A DBA who only has MAINTAIN can't do either one. And the first
option means weakening a protection that the running workload depends
on, just to get a maintenance command to start. That seems backwards
for a command whose whole point is to do the job of VACUUM FULL without
disrupting the workload.

> Antonin Houska has a patch which would probably benefit from your
> review. https://postgr.es/m/108776.1784105248@localhost

Thanks for pointing me to Antonin's patch. I don't think it helps here
though. It keeps REPACK from holding an XID, so REPACK stops blocking
other people. But the worker is on the receiving end. It still has to
wait for whatever regular transactions are running, and that is where
the inherited lock_timeout hits it. The transaction in my repro above
is just a plain INSERT.

I went through the set to be sure. The wait itself is in
SnapBuildWaitSnapshot(), and that function is unchanged across all eight
patches. I do want to look at his work properly, but I'll do that on his
own thread rather than here.

Thanks,
Shihao