Feedback-ID: ie3de48e3:Fastmail
Date: Thu, 17 Sep 2026 12:56:43 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: shihao zhong <zhong950419@gmail.com>
Cc: Chao Li <li.evan.chao@gmail.com>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: REPACK (CONCURRENTLY) decoding worker is canceled by lock_timeout
Message-ID: <aqvFhQOMJsxeYFaD@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAGRkXqS5B3oe305LxByJt3Hh5-fOUU_dRCsR6w9TuyCPXDVz6Q@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aqvFhQOMJsxeYFaD%40alvherre.pgsql>
Precedence: bulk

On 2026-Sep-12, shihao zhong wrote:

> 1. The comment in RepackWorkerMain() now says why only lock_timeout and
> transaction_timeout are passed.  lock_timeout is armed by any lock wait,
> and transaction_timeout by StartTransaction().  Both can fire in the
> worker: the wait for older transactions in the snapshot builder is a
> lock wait, and the worker's transaction spans the whole command.
> statement_timeout and idle_in_transaction_session_timeout are only
> armed by the command loop in PostgresMain(), which a background worker
> never runs.  I hope that answers Chao's question without touching
> autovacuum.

Thanks.  I pushed the patch, but didn't keep the detailed comment.  I
don't think we need to explain the details of how these timeouts work in
this comment, and it may be easy for the comment to become outdated.

I added the word "relevant" to the comment, to try and satisfy Chao's
concern: a reader of this code trying to add a new worker will have to
figure out what does "relevant" mean in whatever case they're trying to
implement :-)

> 2. A lock_timeout on the table owner role does not reach the worker,
> and one set in the REPACK session does.  It fails on unpatched master
> and passes with 0001.

Did you mean to reference the proposed new test in this paragraph?  I
think so, but the text says something else.  (I don't really know what
the text tries to convey).  Anyway, I didn't push this test either,
because I think it's likely to fail randomly on slow buildfarm machines,
so we would have to adjust the timeouts, and that becomes a whack-a-mole
game that I'm not very interested in playing at this point.

Thanks for the report and fix,

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are."  -- Charles J. Sykes' advice to teenagers