]> git.ipfire.org Git - thirdparty/squid.git/commit
Streamline Store swap metadata handling (#1067)
authorEduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Thu, 14 Jul 2022 11:53:28 +0000 (11:53 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Thu, 14 Jul 2022 17:28:23 +0000 (17:28 +0000)
commitd448e1eb9db0c150f88210182a4fbddb81109bdf
tree3e8ff20b511295a2fe5c73207f40457ee0df5728
parent9dc39e0e8aaa1814c418e9f97f666149e5ca9cfc
Streamline Store swap metadata handling (#1067)

Store swap metadata consists of a handful of Squid-specific TLV fields.
In 69c01cd, fields handling was converted to use lists of TLV objects.
In 528b2c6, the fields were promoted to a hierarchy of C++ classes, each
representing a specific T in TLV, but the transition was not complete:
While trying to add another class to the hierarchy, we discovered that
it would require violating hierarchy design again, adding another XXX.

While trying to address those hierarchy problems, we concluded that the
hierarchy and, in fact, the TLV lists themselves should be removed:

* A class hierarchy with polymorphic methods works well when the same
  method can be used regardless of the specific object type. In swap
  metadata case, the user code had to know the type (i.e. T in TLV) to
  handle each object correctly because only certain types could be
  processed at each metadata loading stage. Moreover, some processing
  can benefit from using type-specific APIs (e.g., extracting a URL).
  Several `switch (type)` statements (and XXXs about doing something a
  method should not be doing) illustrate that design conflict.

* A class hierarchy without polymorphic methods can still be useful if
  leaf objects can reuse a lot of parent code. However, StoreMeta class
  had virtually no code reusable by its derived classes -- it was a
  collection of static methods and their equivalents. Metadata leaf
  classes were essentially used as global functions.

* Creating a list is useful when the listed objects are traversed/used
  multiple times. In swap metadata case, each TLV object is effectively
  used once: Official code allocates each object, copies data into it,
  and then uses the object either to interpret the just-read value or to
  write just-copied bytes into another buffer. The only (minor)
  exception to that pattern was when the list was also scanned to
  calculate the total buffer size, but that extra scan is unnecessary
  and undesirable, especially given modern serialization techniques.

This change converts the problematic StoreMeta class hierarchy into a
few stand-alone functions. The SwapMetaView class implements a view of
the serialized metadata, significantly reducing allocations and copying.
Metadata parser creates a cheap view for the being-parsed field, with
view lifetime confined to that single parsing loop iteration.

Also removed unused STORE_META_* type IDs, addressing several related
XXXs and TODOs. This change was necessary to let the compiler tell us
when we forgot a swap metadata type in a `switch (type)` statement. No
changes to type IDs and their deficient assignment algorithm.

This change attempts to preserve arbitrary metadata validation decisions
that do not violate the overall design, except the following two:

* More problematic Store entries may now be rejected due to conversion
  of serious validation errors (e.g., framing problems) to exceptions.
  This change decreases the risks of infinite metadata parsing loops and
  invalid entry acceptance.

* Some error messages are now logged at level 1 instead of 0.

The Rock-specific ZeroedSlot() check was moved to Rock. Unlike Rock
fixed-size one-file storage, UFS-based cache_dir files are not zeroed.
48 files changed:
doc/debug-messages.dox
doc/debug-sections.txt
src/Makefile.am
src/SquidMath.h
src/StoreClient.h
src/StoreMeta.cc [deleted file]
src/StoreMeta.h [deleted file]
src/StoreMetaMD5.cc [deleted file]
src/StoreMetaMD5.h [deleted file]
src/StoreMetaObjSize.h [deleted file]
src/StoreMetaSTD.cc [deleted file]
src/StoreMetaSTD.h [deleted file]
src/StoreMetaSTDLFS.cc [deleted file]
src/StoreMetaSTDLFS.h [deleted file]
src/StoreMetaURL.cc [deleted file]
src/StoreMetaURL.h [deleted file]
src/StoreMetaUnpacker.cc [deleted file]
src/StoreMetaUnpacker.h [deleted file]
src/StoreMetaVary.cc [deleted file]
src/StoreMetaVary.h [deleted file]
src/StoreSwapLogData.h
src/debug/Messages.h
src/defines.h
src/fs/rock/RockHeaderUpdater.cc
src/fs/rock/RockHeaderUpdater.h
src/fs/rock/RockRebuild.cc
src/store.cc
src/store/Makefile.am
src/store/SwapMeta.cc [new file with mode: 0644]
src/store/SwapMeta.h [new file with mode: 0644]
src/store/SwapMetaIn.cc [new file with mode: 0644]
src/store/SwapMetaIn.h [new file with mode: 0644]
src/store/SwapMetaOut.cc [new file with mode: 0644]
src/store/SwapMetaOut.h [new file with mode: 0644]
src/store/SwapMetaView.cc [new file with mode: 0644]
src/store/SwapMetaView.h [new file with mode: 0644]
src/store_client.cc
src/store_key_md5.cc
src/store_key_md5.h
src/store_rebuild.cc
src/store_rebuild.h
src/store_swapmeta.cc [deleted file]
src/tests/Stub.am
src/tests/stub_StoreMeta.cc [deleted file]
src/tests/stub_libstore.cc
src/tests/stub_store_swapout.cc [deleted file]
src/tests/testStore.cc
src/tests/testStore.h