Adds bytes counting as a trigger to the cache_promote LRU

doc/admin-guide/plugins/cache_promote.en.rst

@@ -21,7 +21,7 @@ Cache Promote Plugin
 ********************
 
 The :program:`cache_promote` plugin provides a means to control when an object should
-be allowed to enter the cache. This is orthogonal from normal Cache-Control
+be allowed to enter the cache. This is orthogonal from normal ``Cache-Control``
 directives, providing a different set of policies to apply. The typical use
 case for this plugin is when you have a very large data set, where you want to
 avoid churning the ATS cache for the long tail content.
@@ -50,7 +50,13 @@ If :option:`--policy` is set to ``lru`` the following options are also available
 
 .. option:: --hits
 
-   The minimum number of hits before promotion.
+   The minimum number of requests before promotion.
+
+.. option:: --bytes
+
+   In addition to requests, also count bytes that are cache misses. If specified,
+   default is ``0``, whichever triggers first of bytes and requests (hits) will
+   cause promotion.
 
 .. option:: --buckets
 

plugins/cache_promote/cache_promote.cc

@@ -25,6 +25,7 @@
 #include "configs.h"
 
 const char *PLUGIN_NAME = "cache_promote";
+int TXN_ARG_IDX;
 
 // This has to be a global here. I tried doing a classic singleton (with a getInstance()) in the PolicyManager,
 // but then reloading the DSO does not work. What happens is that the old singleton is stil there, even though
@@ -47,7 +48,7 @@ cont_handle_policy(TSCont contp, TSEvent event, void *edata)
   PromotionConfig *config = static_cast<PromotionConfig *>(TSContDataGet(contp));
 
   switch (event) {
-  // Main HOOK
+  // After the cache lookups check if it should be promoted on cache misses
   case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE:
     if (!TSHttpTxnIsInternal(txnp)) {
       int obj_status;
@@ -60,28 +61,43 @@ cont_handle_policy(TSCont contp, TSEvent event, void *edata)
             TSDebug(PLUGIN_NAME, "cache-status is %d, and leaving cache on (promoted)", obj_status);
           } else {
             TSDebug(PLUGIN_NAME, "cache-status is %d, and turning off the cache (not promoted)", obj_status);
+            if (config->getPolicy()->countBytes()) {
+              // Need to schedule this continuation for read-response-header-hook as well.
+              TSHttpTxnHookAdd(txnp, TS_HTTP_READ_RESPONSE_HDR_HOOK, contp);
+              // This is needed to make sure that we free any data retained in the TXN slot even if the
+              // transaction is terminated early.
+              TSHttpTxnHookAdd(txnp, TS_HTTP_TXN_CLOSE_HOOK, contp);
+            }
             TSHttpTxnServerRespNoStoreSet(txnp, 1);
           }
           break;
         default:
           // Do nothing, just let it handle the lookup.
           TSDebug(PLUGIN_NAME, "cache-status is %d (hit), nothing to do", obj_status);
 
-          if (config->getPolicy()->stats_enabled) {
-            TSStatIntIncrement(config->getPolicy()->cache_hits_id, 1);
+          if (config->getPolicy()->_stats_enabled) {
+            TSStatIntIncrement(config->getPolicy()->_cache_hits_id, 1);
           }
           break;
         }
       }
-
-      if (config->getPolicy()->stats_enabled) {
-        TSStatIntIncrement(config->getPolicy()->total_requests_id, 1);
+      if (config->getPolicy()->_stats_enabled) {
+        TSStatIntIncrement(config->getPolicy()->_total_requests_id, 1);
       }
     } else {
       TSDebug(PLUGIN_NAME, "request is an internal (plugin) request, implicitly promoted");
     }
     break;
 
+  // This is the event when we want to count the bytes cache miss as well as hits
+  case TS_EVENT_HTTP_READ_RESPONSE_HDR:
+    config->getPolicy()->addBytes(txnp);
+    break;
+
+  case TS_EVENT_HTTP_TXN_CLOSE:
+    config->getPolicy()->cleanup(txnp);
+    break;
+
   // Should not happen
   default:
     TSDebug(PLUGIN_NAME, "unhandled event %d", static_cast<int>(event));
@@ -110,7 +126,13 @@ TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
     return TS_ERROR;
   }
 
-  TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized");
+  // Reserve a TXN slot for storing the calculated URL hash key
+  if (TS_SUCCESS != TSUserArgIndexReserve(TS_USER_ARGS_TXN, PLUGIN_NAME, "cache_promote URL hash key", &TXN_ARG_IDX)) {
+    strncpy(errbuf, "[tsremap_init] - Failed to reserve the TXN user argument slot", errbuf_size - 1);
+    return TS_ERROR;
+  }
+
+  TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized, TXN_IDX = %d", TXN_ARG_IDX);
   return TS_SUCCESS; /* success */
 }
 

plugins/cache_promote/chance_policy.h

@@ -29,7 +29,7 @@ class ChancePolicy : public PromotionPolicy
   bool doPromote(TSHttpTxn /* txnp ATS_UNUSED */) override
   {
     TSDebug(PLUGIN_NAME, "ChancePolicy::doPromote(%f)", getSample());
-    incrementStat(promoted_id, 1);
+    incrementStat(_promoted_id, 1);
 
     return true;
   }
@@ -51,9 +51,9 @@ class ChancePolicy : public PromotionPolicy
   {
     std::string_view remap_identifier                 = remap_id;
     const std::tuple<std::string_view, int *> stats[] = {
-      {"cache_hits", &cache_hits_id},
-      {"promoted", &promoted_id},
-      {"total_requests", &total_requests_id},
+      {"cache_hits", &_cache_hits_id},
+      {"promoted", &_promoted_id},
+      {"total_requests", &_total_requests_id},
     };
 
     if (nullptr == remap_id) {

plugins/cache_promote/configs.cc

@@ -26,12 +26,13 @@
 // to add to this list, making them more modular.
 static const struct option longopt[] = {
   {const_cast<char *>("policy"), required_argument, nullptr, 'p'},
+  {const_cast<char *>("stats-enable-with-id"), required_argument, nullptr, 'e'},
   // This is for both Chance and LRU (optional) policy
   {const_cast<char *>("sample"), required_argument, nullptr, 's'},
   // For the LRU policy
   {const_cast<char *>("buckets"), required_argument, nullptr, 'b'},
   {const_cast<char *>("hits"), required_argument, nullptr, 'h'},
-  {const_cast<char *>("stats-enable-with-id"), required_argument, nullptr, 'e'},
+  {const_cast<char *>("bytes"), required_argument, nullptr, 'B'},
   {const_cast<char *>("label"), required_argument, nullptr, 'l'},
   // EOF
   {nullptr, no_argument, nullptr, '\0'},
@@ -70,7 +71,7 @@ PromotionConfig::factory(int argc, char *argv[])
         return false;
       } else {
         if (_policy && _policy->stats_add(optarg)) {
-          _policy->stats_enabled = true;
+          _policy->_stats_enabled = true;
           TSDebug(PLUGIN_NAME, "stats collection is enabled");
         }
       }

plugins/cache_promote/lru_policy.cc

@@ -16,12 +16,49 @@
   limitations under the License.
 */
 #include <unistd.h>
+#include <cinttypes>
 
 #include "lru_policy.h"
 
 #define MINIMUM_BUCKET_SIZE 10
 static LRUEntry NULL_LRU_ENTRY; // Used to create an "empty" new LRUEntry
 
+// Initialize the LRU hash key from the TXN's URL
+bool
+LRUHash::initFromUrl(TSHttpTxn txnp)
+{
+  bool ret     = false;
+  TSMLoc c_url = TS_NULL_MLOC;
+  TSMBuffer reqp;
+  TSMLoc req_hdr;
+
+  if (TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &reqp, &req_hdr)) {
+    return false;
+  }
+
+  if (TS_SUCCESS == TSUrlCreate(reqp, &c_url)) {
+    if (TS_SUCCESS == TSHttpTxnCacheLookupUrlGet(txnp, reqp, c_url)) {
+      int url_len = 0;
+      char *url   = TSUrlStringGet(reqp, c_url, &url_len);
+
+      if (url && url_len > 0) {
+        SHA_CTX sha;
+
+        SHA1_Init(&sha);
+        TSDebug(PLUGIN_NAME, "LRUHash::initFromUrl(%.*s%s)", url_len > 100 ? 100 : url_len, url, url_len > 100 ? "..." : "");
+        SHA1_Update(&sha, url, url_len);
+        SHA1_Final(_hash, &sha);
+        TSfree(url);
+        ret = true;
+      }
+    }
+    TSHandleMLocRelease(reqp, TS_NULL_MLOC, c_url);
+  }
+  TSHandleMLocRelease(reqp, TS_NULL_MLOC, req_hdr);
+
+  return ret;
+}
+
 LRUPolicy::~LRUPolicy()
 {
   TSDebug(PLUGIN_NAME, "LRUPolicy DTOR");
@@ -52,6 +89,9 @@ LRUPolicy::parseOption(int opt, char *optarg)
   case 'h':
     _hits = static_cast<unsigned>(strtol(optarg, nullptr, 10));
     break;
+  case 'B':
+    _bytes = static_cast<int64_t>(strtoll(optarg, nullptr, 10));
+    break;
   case 'l':
     _label = optarg;
     break;
@@ -72,101 +112,126 @@ LRUPolicy::doPromote(TSHttpTxn txnp)
 {
   LRUHash hash;
   LRUMap::iterator map_it;
-  char *url   = nullptr;
-  int url_len = 0;
-  bool ret    = false;
-  TSMBuffer request;
-  TSMLoc req_hdr;
-
-  if (TS_SUCCESS == TSHttpTxnClientReqGet(txnp, &request, &req_hdr)) {
-    TSMLoc c_url = TS_NULL_MLOC;
+  bool ret = false;
 
-    // Get the cache key URL (for now), since this has better lookup behavior when using
-    // e.g. the cachekey plugin.
-    if (TS_SUCCESS == TSUrlCreate(request, &c_url)) {
-      if (TS_SUCCESS == TSHttpTxnCacheLookupUrlGet(txnp, request, c_url)) {
-        url = TSUrlStringGet(request, c_url, &url_len);
-        TSHandleMLocRelease(request, TS_NULL_MLOC, c_url);
-      }
-    }
-    TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);
-  }
-
-  // Generally shouldn't happen ...
-  if (!url) {
+  if (!hash.initFromUrl(txnp)) {
     return false;
   }
 
-  TSDebug(PLUGIN_NAME, "LRUPolicy::doPromote(%.*s%s)", url_len > 100 ? 100 : url_len, url, url_len > 100 ? "..." : "");
-  hash.init(url, url_len);
-  TSfree(url);
-
   // We have to hold the lock across all list and hash access / updates
   TSMutexLock(_lock);
 
   map_it = _map.find(&hash);
   if (_map.end() != map_it) {
+    auto &[map_key, map_val]             = *map_it;
+    auto &[val_key, val_hits, val_bytes] = *(map_it->second);
+
+    // This is beacuse compilers before gcc 8 aren't smart enough to ignore the unused structured bindings
+    (void)val_key;
+
     // We have an entry in the LRU
     TSAssert(_list_size > 0); // mismatch in the LRUs hash and list, shouldn't happen
-    incrementStat(lru_hit_id, 1);
-    if (++(map_it->second->second) >= _hits) {
+    incrementStat(_lru_hit_id, 1);
+    if (++val_hits >= _hits || (_bytes > 0 && val_bytes > _bytes)) {
       // Promoted! Cleanup the LRU, and signal success. Save the promoted entry on the freelist.
       TSDebug(PLUGIN_NAME, "saving the LRUEntry to the freelist");
-      _freelist.splice(_freelist.begin(), _list, map_it->second);
+      _freelist.splice(_freelist.begin(), _list, map_val);
       ++_freelist_size;
       --_list_size;
-      _map.erase(map_it->first);
-      incrementStat(promoted_id, 1);
-      incrementStat(freelist_size_id, 1);
-      decrementStat(lru_size_id, 1);
+      _map.erase(map_key);
+      incrementStat(_promoted_id, 1);
+      incrementStat(_freelist_size_id, 1);
+      decrementStat(_lru_size_id, 1);
       ret = true;
     } else {
       // It's still not promoted, make sure it's moved to the front of the list
-      TSDebug(PLUGIN_NAME, "still not promoted, got %d hits so far", map_it->second->second);
-      _list.splice(_list.begin(), _list, map_it->second);
+      TSDebug(PLUGIN_NAME, "still not promoted, got %d hits so far and %" PRId64 " bytes", val_hits, val_bytes);
+      _list.splice(_list.begin(), _list, map_val);
     }
   } else {
     // New LRU entry for the URL, try to repurpose the list entry as much as possible
-    incrementStat(lru_miss_id, 1);
+    incrementStat(_lru_miss_id, 1);
     if (_list_size >= _buckets) {
       TSDebug(PLUGIN_NAME, "repurposing last LRUHash entry");
       _list.splice(_list.begin(), _list, --_list.end());
-      _map.erase(&(_list.begin()->first));
-      incrementStat(lru_vacated_id, 1);
+      _map.erase(&(std::get<0>(*_list.begin()))); // Get the hash from the first list element
+      incrementStat(_lru_vacated_id, 1);
     } else if (_freelist_size > 0) {
       TSDebug(PLUGIN_NAME, "reusing LRUEntry from freelist");
       _list.splice(_list.begin(), _freelist, _freelist.begin());
       --_freelist_size;
       ++_list_size;
-      incrementStat(lru_size_id, 1);
-      decrementStat(freelist_size_id, 1);
+      incrementStat(_lru_size_id, 1);
+      decrementStat(_freelist_size_id, 1);
     } else {
       TSDebug(PLUGIN_NAME, "creating new LRUEntry");
       _list.push_front(NULL_LRU_ENTRY);
       ++_list_size;
-      incrementStat(lru_size_id, 1);
+      incrementStat(_lru_size_id, 1);
     }
     // Update the "new" LRUEntry and add it to the hash
-    _list.begin()->first          = hash;
-    _list.begin()->second         = 1;
-    _map[&(_list.begin()->first)] = _list.begin();
+    *_list.begin()                       = {hash, 1, 0};
+    _map[&(std::get<0>(*_list.begin()))] = _list.begin();
   }
 
   TSMutexUnlock(_lock);
 
+  // If we didn't promote, and we want to count bytes, save away the calculated hash for later use
+  if (false == ret && countBytes()) {
+    TSUserArgSet(txnp, TXN_ARG_IDX, static_cast<void *>(new LRUHash(hash)));
+  } else {
+    TSUserArgSet(txnp, TXN_ARG_IDX, nullptr);
+  }
+
   return ret;
 }
 
+void
+LRUPolicy::addBytes(TSHttpTxn txnp)
+{
+  LRUHash *hash = static_cast<LRUHash *>(TSUserArgGet(txnp, TXN_ARG_IDX));
+
+  if (hash) {
+    LRUMap::iterator map_it;
+
+    // We have to hold the lock across all list and hash access / updates
+    TSMutexLock(_lock);
+    map_it = _map.find(hash);
+    if (_map.end() != map_it) {
+      TSMBuffer resp;
+      TSMLoc resp_hdr;
+
+      if (TS_SUCCESS == TSHttpTxnServerRespGet(txnp, &resp, &resp_hdr)) {
+        TSMLoc field_loc = TSMimeHdrFieldFind(resp, resp_hdr, TS_MIME_FIELD_CONTENT_LENGTH, TS_MIME_LEN_CONTENT_LENGTH);
+
+        if (field_loc) {
+          auto &[val_key, val_hits, val_bytes] = *(map_it->second);
+          int64_t cl                           = TSMimeHdrFieldValueInt64Get(resp, resp_hdr, field_loc, -1);
+
+          // This is beacuse compilers before gcc 8 aren't smart enough to ignore the unused structured bindings
+          (void)val_key, (void)val_hits;
+
+          val_bytes += cl;
+          TSDebug(PLUGIN_NAME, "Added %" PRId64 " bytes for LRU entry", cl);
+          TSHandleMLocRelease(resp, resp_hdr, field_loc);
+        }
+        TSHandleMLocRelease(resp, TS_NULL_MLOC, resp_hdr);
+      }
+    }
+    TSMutexUnlock(_lock);
+  }
+}
+
 bool
 LRUPolicy::stats_add(const char *remap_id)
 
 {
   std::string_view remap_identifier                 = remap_id;
   const std::tuple<std::string_view, int *> stats[] = {
-    {"cache_hits", &cache_hits_id}, {"freelist_size", &freelist_size_id},
-    {"lru_size", &lru_size_id},     {"lru_hit", &lru_hit_id},
-    {"lru_miss", &lru_miss_id},     {"lru_vacated", &lru_vacated_id},
-    {"promoted", &promoted_id},     {"total_requests", &total_requests_id},
+    {"cache_hits", &_cache_hits_id}, {"freelist_size", &_freelist_size_id},
+    {"lru_size", &_lru_size_id},     {"lru_hit", &_lru_hit_id},
+    {"lru_miss", &_lru_miss_id},     {"lru_vacated", &_lru_vacated_id},
+    {"promoted", &_promoted_id},     {"total_requests", &_total_requests_id},
   };
 
   if (nullptr == remap_id) {