Avoid store_client.cc "entry->swap_filen > -1 || entry->swappingOut()" asserts.

A client may hit on an incomplete shared memory cache entry. Such entry is
fully backed by the shared memory cache, but the copy of its data in local RAM
may be trimmed. When that trimMemory() happens, StoreEntry::storeClientType()
assumes DISK_CLIENT due to positive inmem_lo, and the store_client constructor
asserts upon discovering that there is no disk backing.

To improve shared cache effectiveness for "being cached" entries, we need to
prevent local memory trimming while the shared cache entry is being filled
(possibly by another worker, so this is far from trivial!) or, better, stop
using the local memory for entries feeding off the shared memory cache. The
latter would also require revising DISK_CLIENT designation to include entries
backed by a shared memory cache.

diff --git a/src/store.cc b/src/store.cc
index 62371cbdd39cedb16347da4a07b2a45d3b8775e1..6bef925fbeb4be4b5d644345a35186c8961ad6f2 100644 (file)
--- a/src/store.cc
+++ b/src/store.cc
@@ -1536,15 +1536,20 @@ StoreEntry::validToSend() const
     if (!mem_obj) // not backed by a memory cache and not collapsed
         return 0;
 
-    if (mem_obj->memCache.index >= 0) // backed by a shared memory cache
-        return 1;
-
     // StoreEntry::storeClientType() assumes DISK_CLIENT here, but there is no
-    // disk cache backing so we should not rely on the store cache at all. This
-    // is wrong for range requests that could feed off nibbled memory (XXX).
-    if (mem_obj->inmem_lo) // in local memory cache, but got nibbled at
+    // disk cache backing that store_client constructor will assert. XXX: This
+    // is wrong for range requests (that could feed off nibbled memory) and for
+    // entries backed by the shared memory cache (that could, in theory, get
+    // nibbled bytes from that cache, but there is no such "memoryIn" code).
+    if (mem_obj->inmem_lo) // in memory cache, but got nibbled at
         return 0;
 
+    // The following check is correct but useless at this position. TODO: Move
+    // it up when the shared memory cache can either replenish locally nibbled
+    // bytes or, better, does not use local RAM copy at all.
+    // if (mem_obj->memCache.index >= 0) // backed by a shared memory cache
+    //    return 1;
+
     return 1;
 }