]> git.ipfire.org Git - thirdparty/squid.git/commit
Stop (ab)using Transient entry flags for CF requirement mngmt (#1127)
authorAlex Rousskov <rousskov@measurement-factory.com>
Fri, 2 Sep 2022 17:14:12 +0000 (17:14 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Sat, 3 Sep 2022 18:41:32 +0000 (18:41 +0000)
commit9358e99f998ace9c4c7a21d510432dde5b7f9cca
treefd1a92d45d990858b58f4e8bc39c860e0507f292
parent911833fcbfae14e16c30adeb5bb97e763451018f
Stop (ab)using Transient entry flags for CF requirement mngmt (#1127)

The ENTRY_REQUIRES_COLLAPSING flag was used for relaying "hitting this
entry carries Collapsed Forwarding risks right now" info to other
workers. That flag is not necessary to relay that info because remote
workers already know whether the entry is attached to one of the shared
cache stores. Moreover, relaying that flag correctly is difficult and
would require a different implementation:

* The flag is changed in relatively high-level Store code, possibly
  _after_ we stopped writing, when the transients entry is already
  read-only. This bug leads to an isWriter() assertion in
  Transients::clearCollapsingRequirement() called via a quick or aborted
  StoreEntry::startWriting().

* Basics.flags is not an atomic field. It lacks an "[app]" mark -- its
  readers will not get a reliable value due to race conditions when
  Squid will need to update the Transient entry flags in "append" mode.

* The flag is changed assuming that simply sending stuff to disk is
  sufficient for other workers to read it. Since 18102f7, this is
  inaccurate because the store index is updated only after the slot is
  written to disk.

Remote workers now use Store attachment info to set the local StoreEntry
object flag, ignoring the no-longer-used Transients entry flags.

The ENTRY_REQUIRES_COLLAPSING flag is still used to tell transactions
which StoreEntry objects are subject to CF risks. It would be nice to
use the presence of MemObject::reply_ to relay the same info for
store-unattached entries (and attachment info for others), but we could
not find a way to do that (during commit d2a6dcb work and then again
during this project) without a lot of essentially out of scope rewrites.

Also removed misleading Ipc::StoreMapAnchor::exportInto() debugging.
src/Transients.cc
src/Transients.h
src/ipc/StoreMap.cc
src/store.cc
src/store/Controller.cc
src/store/Controller.h
src/tests/stub_libstore.cc