On Wed, Sep 16, 2026 at 5:43 AM Masahiko Sawada <[email protected]> wrote: > > I looked at the back-branch ones and I think they have a problem that > the HEAD patch doesn't have. The back branches return early on subxact > commit: > > + if (isCommit) > + return; > > So once the subxact that acquired the slot commits, > MyReplicationSlotSubId keeps the id of a subxact that is already gone. > Subxact ids restart at TopSubTransactionId in every transaction since > StartTransaction() resets currentSubTransactionId, so the same id > comes around again.It's not a problem for the core use cases, but if > there is an external SQL function that keeps the slot when the > transaction ends, that stale id can match a completely unrelated > subxact in a later transaction and we release a slot that subxact > never acquired. > > What bothers me is that this pattern works today on all branches. > While I guess it's not a good programming practice, we don't restrict > such use cases. So I think it's not a case of not supporting that > usage, it's a behavior change we would be introducing in a minor > release. > > That makes me want to reconsider how we split the patches. IIUC the > handoff mechanism that the master patch implements is to (1) keep > MyReplicationSlotSubId from going stale and (2) give the slot a new > guarantee, that the slot is released if an ancestor subxact aborts, > which nothing does today. (2) is the part that broadens what an > extension can do whereas (1) is just cleaning up after the variable we > added. I think we can fix the reported problem only with (1) even > without (2). So I guess it would be cleaner to do (1) for all > branches, and do (2) only for master. As for (1), we can have a > function like AtEOXact_ReplicationSlot() just clearing > MyReplicationSlotSubId. For (2), we can prepare a separate patch that > implements the handoff mechanism (possibly with a WARNING or DEBUG > message) with the regression tests, if we want to support these cases. > > It seems confusing and I might be too pessimistic as this is all about > hypothetical cases that might not exist, but I'd like to keep the > back-branch fix to the smallest thing that fixes only the reported > problem while not changing other current behaviors. >
Yeah, we can do a minimal fix for back-branches on the lines you are suggesting but OTOH, I think we are over worried about the hypothetical cases. I feel there is no harm in keeping the HEAD and back-branches code/behavior same, in the worst case, if we get any report, we can address keeping the actual usage in mind. -- With Regards, Amit Kapila.
