]> git.ipfire.org Git - thirdparty/squid.git/commit
Concurrent hit misses due to Transients entry creation collision (#686)
authorAlex Rousskov <rousskov@measurement-factory.com>
Fri, 3 Jul 2020 21:06:17 +0000 (21:06 +0000)
committerAmos Jeffries <yadij@users.noreply.github.com>
Tue, 17 Nov 2020 01:10:07 +0000 (14:10 +1300)
commitec500619213ab5dd47d95c38a4823da3a780be0d
tree20105c8363b4df3e9613c170c88c951feeb40897
parent4227b4d4122a974ac4760cb742af94d4bad803a0
Concurrent hit misses due to Transients entry creation collision (#686)

Transients::addEntry() fails when two simultaneous cache hit requests
arrive at two different workers:

    Controller.cc find: e:=msV/0x55c2dbdac7f0*0 check failed: slot
    exception location: Transients.cc(236) addEntry

The failure leads to a cache miss for one of the requests. The miss
transaction will overwrite the already cached entry, possibly creating
more misses due to long write locks used by the disk cache. The problem
does not affect collapsed requests. It affects regular cache hits.

The failure happens because two requests are trying to create a
Transients entry at about the same time. Only one can succeed. With the
current ReadWriteLock-based design, some contention is unavoidable, but
the contention window was rather large/long:

* The critical section essentially started when Controller::find() (in
  each request worker) checked Transients and did not find a matching
  entry there. That failed lookup put each request into "create a new
  Transients entry" mode.

* A worker exited that critical section when openForWriting() in
  Transients::addEntry() returned (with a write lock to the
  ready-to-be-filled entry for one of the two requests and with a
  failure for the other).

This change reduces the contention window to the bare minimum necessary
to create and fill a Transients entry inside openOrCreateForReading().
In the micro-tests that exposed this problem, the probability of a
failure is reduced from more than 70% to less than 3%. YMMV.

Also split addEntry() into two directional methods because while they
are structured similarly they actually have almost no common code now.
src/Transients.cc
src/Transients.h
src/ipc/StoreMap.cc
src/ipc/StoreMap.h