[squid-dev] [PATCH] Purge cache entries in SMP-aware caches

[Eduard Bagdasaryan]

Hello,

I am attaching an improved version of the patch posted before.
It is based on v4 r15081. What was fixed:

1. Correctly purge StoreEntries, 'disconnected' from the Store
     (unlinking by a valid cache_key in these cases).
2. Do not release just created keyless StoreEntries. This caused
     several problems, such as calling key-dependent methods with
     nil key, setting the private key twice and setting the private key
     to be overwritten by the public key later in some other callers.
3. Fixed tests/testRock test case, to conform to the
     "do not overwrite an existing (and identical) disk entry"
     requirement.
4. Documented several methods and eliminated some code
     duplication (hasMemStore() and hasTransients() helper
     methods). More work needed in this direction.


Eduard.

On 01.06.2017 19:27, Eduard Bagdasaryan wrote:
> Hello,
>
> This patch fixes Squid bug 4505.
>
> Before this change, SMP Squid with SMP-aware caches sometimes
> does not purge cache entries.
>
> When Squid finds a requested entry in the memory cache, it does not
> check whether the same entry is also stored in a cache_dir. The
> StoreEntry object may become associated with its store entry in the
> memory cache but not with its store entry on disk. This inconsistency
> causes two known problems:
>
> 1. Squid may needlessly swap out the memory hit to disk, either
>    overwriting an existing (and identical) disk entry or, worse,
>    creating a duplicate entry on another disk. In the second case, the
>    two disk entries are not synchronized and may eventually start to
>    differ if one of them is removed or updated.
>
> 2. Squid may not delete a stale disk entry when needed, violating
>    various HTTP MUSTs, and eventually serving stale [disk] cache entries
>    to clients.
>
> Another purging problem is not caused by the above inconsistency:
>
> 3. A DELETE request or equivalent may come for the entry which is still
>    locked for writing. Squid fails to get a lock for such an entry (in
>    order to purge it) and the entry remains in disk and/or memory cache.
>
> To solve the first two problems:
>
> * StoreEntry::mayStartSwapout() now avoids needless swapouts by checking
>   whether StoreEntry was fully loaded, is being loaded, or could have
>   been loaded from disk.
>
> To fix problem #3:
>
> * A new Store::Controller::unlinkByKey() method purges (or marks for
>   deletion if purging is impossible) all the matching store entries,
>   without loading the StoreEntry information from stores. The lack of
>   StoreEntry creation reduces waste of resources (the StoreEntry object
>   would have to be deleted anyway) _and_ allows us to mark being-created
>   entries (that are locked for writing and, hence, cannot be loaded into
>   a StoreEntry object).
>
> Note that even if Squid properly avoids storing duplicate disk entries,
> some cache_dir manipulations by humans and Squid crashes may lead to
> such duplicates being present.  This patch leaves dealing with potential
> duplicates out of scope except it guarantees that if an entry is
> deleted, then all [possible] duplicates are deleted as well.
>
> Also added StoreEntry helper methods to prevent direct manipulation of
> individual disk-related data members (swap_dirn, swap_filen, and
> swap_status). These methods help keep these related data members in a
> coherent state and minimize code duplication.
>
> Also tightened rules around several Storage methods: A storage method
> must not be given a StoreEntry object unless that object belongs to that
> storage.
>
> Also do not mark Transients entry as "waitingToBeFreed" when the entry
> is simply closed for writing. This flag is used to mark locked entries
> for deletion and should not be used for "completed" transient entries to
> avoid confusion.
>
> XXX: SMP cache purges may continue to malfunction when the Transients
> table is missing. Currently, Transients are created only when the
> collapsed_forwarding is on. After Squid bug 4579 is fixed, every public
> StoreEntry will have the corresponding Transients entry and vice versa,
> extending these fixes to all SMP environments.
>
> Since this work started long ago and is v4-based, I am attaching the
> existing v4 patch for review now and start converting it to v5 version.
> I plan to post final v5 version after the review is over.
>
>
> Thanks,
> Eduard.
>
>
>
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20170713/6f3c001c/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SQUID-175-methods-do-not-invalidate-v4-t17.patch
Type: text/x-patch
Size: 187696 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20170713/6f3c001c/attachment-0001.bin>