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.". > 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. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
