On Thu, Oct 15, 2026 at 3:31 PM Zhijie Hou <[email protected]> wrote: > > Hi, > > On Tue, Oct 13, 2026 at 6:04 PM Dilip Kumar <[email protected]> wrote: > > > > I merged v80-0001 and v80-0002 and adjusted the commit message and > > test accordingly; I also included Amit's and Hou-san's code > > refactoring. Rebased Nisha's V80_2-0003 on top of that as 0002, also > > merged Shveta's datestyle fix with some modification into the 0002 > > Thanks for updating the patches. > > While reviewing the follow-up patch to error out the worker when it fails to > acquire the CLT lock, I noticed that all the checks in the DDL and the apply > worker are done without checking whether the two_phase option is enabled. > Since > the issue only exists when the subscription has prepared a transaction, I > think > we shall skip these checks when two_phase is disabled. >
Agreed. In v82-0002, the DDL checks in ALTER SUBSCRIPTION and DROP SUBSCRIPTION now look for prepared transactions only when two_phase is enabled. > In addition, the apply worker currently doesn't check whether a prepared > transaction exists before reporting the ERROR, while we do check in the DDL > path. To make it consistent, I think it would be better to check in the apply > worker as well, this can further narrow down the scope of the ERROR. The > change > should be straightforward, and I think there won't be noticeable overhead: > we'd only scan prepared transactions when lock acquisition conflicts, in which > case the worker will either restart or wait on the lock anyway, so the scan is > unnoticeable by comparison. > Adding the two_phase check in the apply worker makes sense, and I've done that. Without two_phase, the worker now just waits for the lock. But checking whether prepared transactions exist introduces a race condition because one can appear while the worker is waiting. With parallel apply, another worker of the same subscription can prepare the transaction that the waiting command (e.g. TRUNCATE) is blocked on. If the worker found no prepared transaction and started waiting, it would be stuck behind that command, and could never apply the COMMIT PREPARED that releases the lock. So I added only the two_phase check in the worker, and a comment in lock_conflictlog_relation() explaining why there's no prepared-transaction check. -- Thanks, Nisha
