[squid-dev] [PREVIEW] Store API changes blocking bug #7

[Alex Rousskov]

Hello,

    The attached compressed diff represents a 70% complete first step
towards bug #7 fix. This step focus is on fixing "all Stores are the
same" API that forced us to bloat the base Store class with methods
needed only in Store::Root() Controller or other specific storage
classes. Besides unblocking bug 7 development, one of the primary
objectives of this step was to avoid functionality changes (so that they
will be considered for v4.0 inclusion instead of forcing a branching point).

There are a few technical details below, but I am posting this preview
to ask the following high-level questions:

1. Should I move most of the remaining [Ss]tore*.{cc,h} code to src/store/?

2. Should I place most of the moved code in #1 under Store namespace,
and perform other aggressive renamings as needed to polish Store API?

Item #1 will require many changes to #include statements throughout the
code but is kind of incomplete (some might say pointless?) without #2.
Doing #2 will cause lots of changes in the caller code throughout Squid.
Both will significantly delay bug #7 fix due to sheer amount of renaming
work. If there is no strong consensus that these changes should be done
now, then I will _not_ do them, finish the previewed changes, and then
proceed with the actual bug #7 fix.

Also, src/store/Disk.* (i.e. a single cache_dir) kind of clashes with
src/disk.* (low-level disk I/O operations), but I cannot find a better
name for Store::Disk. The old SwapDir name feels rather inadequate for
some cache_dir types, and especially awkward inside the Store namespace:
Store::SwapDir. Better names for Store::Disk and Store::Disks classes
are welcomed.


Please keep the virtually-no-functionality-changes restriction in mind
when answering these questions!



The attached patch polishes class naming and source file locations:

  src/SwapDir.h => src/store/Disk.h (and Controller.h)
  src/SwapDir.cc => src/store/Disk.cc
  src/StoreHashIndex.h => src/store/Disks.h
  src/store_dir.cc => src/store/Controller.cc (and Disks.cc)

The code movement to files in parenthesis is not tracked by bzr because
bzr cannot track file splits, and most of the moved code had to be
splitted across multiple files to untangle various messes. When deciding
what to tell "bzr mv", I picked file pairs that would allow us to track
the most complex, most voluminous code but there is probably no single
correct way to do that.

Again, I tried to preserve the functionality of the moved code (but
without better bzr support it is especially difficult to validate that
all moved code was moved intact).


The Store namespace hierarchy now looks like this:

* Storage: Any storage. Similar to the old Store class, but leaner.

* Controller: Combined memory/disks caches and transients.

* Controlled: Memory cache, disk(s) cache, or transient Storage.

* Disks: All disk caches combined.

* Disk: A single cache_dir Storage.

* Memory: A memory cache.

* Transients: Entries capable of being collapsed for CF.

The last two are not moved/finalized yet, but it should not be too
difficult.

Untested; "make all" works (with hacks) but "make check" fails. This is
PREVIEW!


Thank you,

Alex.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug7-api-fixes-t5.patch.gz
Type: application/gzip
Size: 49079 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20151111/ff557606/attachment-0001.bin>