On 9 June 2018 at 15:41, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Fri, Jun 8, 2018 at 5:27 PM, Simon Riggs <si...@2ndquadrant.com> wrote: >> On 8 June 2018 at 11:33, Amit Kapila <amit.kapil...@gmail.com> wrote: >>> On Fri, Jun 8, 2018 at 1:53 PM, Simon Riggs <si...@2ndquadrant.com> wrote: >>>> >>>> So the attached patch fixes both the bug in the recent commit and the >>>> one I just found by observation of 49bff5300d527, since they are >>>> related. >>>> >>>> StandbyReleaseOldLocks() can sweep in the same way as >>>> ExpireOldKnownAssignedTransactionIds(). >>>> >>> >> >>> How will this avoid the bug introduced by your recent commit >>> (32ac7a11)? After that commit, we no longer consider vacuum's xid in >>> RunningTransactions->oldestRunningXid calculation, so it is possible >>> that we still remove/release its lock on standby earlier than >>> required. >> >> In the past, the presence of an xid was required to prevent removal by >> StandbyReleaseOldLocks(). >> >> The new patch removes that requirement and removes when the xid is >> older than oldestxid >> > > The case I have in mind is: "Say vacuum got xid 600 while truncating, > and then some other random transactions 601,602 starts modifying the > database. At this point, I think the computed value of > RunningTransactions->oldestRunningXid will be 601. Now, on standby > when StandbyReleaseOldLocks sees the lock from transaction 600, it > will remove it which doesn't appear correct to me.".
OK, thanks. Patch attached. >> The normal path of removal is at commit or abort, >> StandbyReleaseOldLocks is used almost never (as originally intended). >> > Okay, but that doesn't prevent us to ensure that whenever used, it > does the right thing. What do you mean? Has anyone argued in favour of doing the wrong thing? I'm playing around with finding a test case to prove this area works, rather than just manual testing. Suggestions welcome. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
execution_ordering_in_running_xact.v1.patch
Description: Binary data
