On Wed, Oct 28, 2026 at 4:34 PM Dilip Kumar <[email protected]> wrote: > > On Wed, Oct 28, 2026 at 2:28 PM shveta malik <[email protected]> wrote: > > > > On Wed, Oct 28, 2026 at 10:50 AM shveta malik <[email protected]> > > wrote: > > > > > > On Tue, Oct 27, 2026 at 5:03 PM Nisha Moond <[email protected]> > > > wrote: > > > > > > > > Please find the updated patches v85. I also incorporated Shveta's doc > > > > suggestion[1] into the v85-0004 patch. > > > > > > Thanks for the patches. You did not post v85-0004, which you are > > > referring to in the above comment. > > > > > > I reviewed 001 mainly. A few comments: > > > > > > > Reviewed 002 and 003, one minor comment in 002, we have error messages > > like below: > > > > + errdetail("Dropping the conflict log table requires a lock that > > prepared transactions of this subscription may be holding."), > > > > + errdetail("Prepared transactions of the subscription may hold a lock > > on its conflict log table."), > > > > The second message uses the ambiguous "its". It would be clearer to > > explicitly refer to the 'subscription’s conflict log table', as in the > > first message. > > > > Suggestion: > > "Prepared transactions of the subscription may hold a lock on the > > subscription’s conflict log table." > > Attached patch set fixes all the comments of 0001 and 0002. It also > includes Nisha's offlist top up patch for 0001 and 0002, along with > some offlist suggestions from Amit. Additionally I have removed this > line "Renamed the conflict log table column remote_origin to > apply_origin." from the commit message, because this change is not on > top of the HEAD, it was just changed over previous version of the same > patch. >
Just one trivial thing, in 002's new change in AlterSubscriptionOwner_internal(), it would be good to change comment slightly from 'while the prepared transaction waits for the apply worker' to 'while we wait for the prepared transaction to be resolved by the apply worker' for better clarity. thanks Shveta
