On Wed, Apr 20, 2016 at 7:39 PM, Andres Freund <andres@anarazel.de> wrote: > > On 2016-04-19 20:27:31 +0530, Amit Kapila wrote: > > On Sun, Apr 17, 2016 at 2:26 AM, Andres Freund <andres@anarazel.de> wrote: > > > > > > On 2016-04-16 16:44:52 -0400, Noah Misch wrote: > > > > That is more controversial than the potential ~2% regression for > > > > old_snapshot_threshold=-1. Alvaro[2] and Robert[3] are okay releasing > > > > that way, and Andres[4] is not. > > > > > > FWIW, I could be kinda convinced that it's temporarily ok, if there'd be > > > a clear proposal on the table how to solve the scalability issue around > > > MaintainOldSnapshotTimeMapping(). > > > > > > > It seems that for read-only workloads, MaintainOldSnapshotTimeMapping() > > takes EXCLUSIVE LWLock which seems to be a probable reason for a > > performance regression. > > Yes, that's the major problem. > > > > Now, here the question is do we need to acquire that lock if xmin is > > not changed since the last time value of > > oldSnapshotControl->latest_xmin is updated or xmin is lesser than > > equal to oldSnapshotControl->latest_xmin? If we don't need it for > > above cases, I think it can address the performance regression to a > > good degree for read-only workloads when the feature is enabled. > > I think the more fundamental issue is that the time->xid mapping is > built at GetSnapshotData() time (via MaintainOldSnapshotTimeMapping()), > and not when xids are assigned. Snapshots are created a lot more > frequently in nearly all use-cases than xids are assigned. That's what > forces the exclusive lock to be in the read path, rather than the write > path. > > What's the reason for this? >
I don't see any particular reason for doing so, but not sure if it will be beneficial in all kind of cases if we build that mapping when xids are assigned. As an example, consider the case where couple of write transactions start at same time and immediately after that a read statement is executed, now for all-those write-transactions we need to take Exclusive lock to build an oldsnaphot entry, whereas with the above optimization suggested by me, it needs to take Exclusive lock just once for read-statement.
