On 14 March 2018 at 13:33, Pavan Deolasee <pavan.deolasee@gmail.com> wrote: > > > On Wed, Mar 14, 2018 at 4:53 PM, Simon Riggs <simon.riggs@2ndquadrant.com> > wrote: >> >> On 14 March 2018 at 04:36, Pavan Deolasee <pavan.deolasee@gmail.com> >> wrote: >> > I wonder if the shortened code path actually leads to >> > heavier contention for EXCLUSIVE lock on the rightmost page, which in >> > turn >> > causes the slowdown. But that's just a theory. Any ideas how to prove or >> > disprove that theory or any other pointers? >> >> Certainly: dropping performance with higher sessions means increased >> contention is responsible. Your guess is likely correct. >> >> Suggest making the patch attempt a ConditionalLock on the target >> block, so if its contended we try from top of index. So basically only >> attempt the optimization if uncontended. This makes sense because in >> many cases if the block is locked it is filling up and the RHS block >> is more likely to change anyway. > > > Hmm. I can try that. It's quite puzzling though that slowing down things > actually make them better.
That's not what is happening though.
The cache path is 1) try to lock cached block, 2) when got lock check relevance, if stale 3) recheck from top
The non-cached path is just 3) recheck from top
The overall path length is longer in the cached case but provides benefit if we can skip step 3 in high % of cases. The non-cached path is more flexible because it locates the correct RHS block, even if it changes dynamically between starting the recheck from top.
So as I noted in one of the previous emails, the revised patch still takes fast path in 98% cases. So it's not clear why the taking steps 1, 2 and 3 in just 2% cases should cause such dramatic slowdown.
If there is enough delay in step 1 then any benefit is lost anyway, so there is no point taking the cached path even when successful, so its better to ignore in that case.
The non-cached path is also going to land on the same page, it just that the _bt_search() will ensure that it doesn't happen quickly. So my understanding that the additional time spent in _bt_search() accidentally reduces contention on the EX lock and ends up improving net performance. I know it sounds weird..