diff --git a/.bleep b/.bleep index e317cd1..6df7774 100644 --- a/.bleep +++ b/.bleep @@ -1 +1 @@ -a1e399e99a67e743d00ba50ba7c3999c25136fe5 \ No newline at end of file +d37f942d73e5b2921f026cf6e2b1f93a216d895b \ No newline at end of file diff --git a/pingora-cache/src/filters.rs b/pingora-cache/src/filters.rs index 5e7e666..6107fc8 100644 --- a/pingora-cache/src/filters.rs +++ b/pingora-cache/src/filters.rs @@ -33,7 +33,6 @@ pub fn request_cacheable(req_header: &ReqHeader) -> bool { /// /// `cache_control` is the parsed [CacheControl] from the response header. It is a standalone /// argument so that caller has the flexibility to choose to use, change or ignore it. -// TODO: vary processing pub fn resp_cacheable( cache_control: Option<&CacheControl>, resp_header: &ResponseHeader, diff --git a/pingora-proxy/src/proxy_cache.rs b/pingora-proxy/src/proxy_cache.rs index f40e083..54ee621 100644 --- a/pingora-proxy/src/proxy_cache.rs +++ b/pingora-proxy/src/proxy_cache.rs @@ -83,30 +83,34 @@ impl HttpProxy { match session.cache.cache_lookup().await { Ok(res) => { if let Some((mut meta, handler)) = res { - // vary logic - // because this branch can be called multiple times in a loop, and we only + // Vary logic + // Because this branch can be called multiple times in a loop, and we only // need to update the vary once, check if variance is already set to - // prevent unnecessary vary lookups + // prevent unnecessary vary lookups. let cache_key = session.cache.cache_key(); if let Some(variance) = cache_key.variance_bin() { - // adhoc double check the variance found is the variance we want + // We've looked up a secondary slot. + // Adhoc double check that the variance found is the variance we want. if Some(variance) != meta.variance() { warn!("Cache variance mismatch, {variance:?}, {cache_key:?}"); session.cache.disable(NoCacheReason::InternalError); break None; } } else { + // Basic cache key; either variance is off, or this is the primary slot. let req_header = session.req_header(); let variance = self.inner.cache_vary_filter(&meta, ctx, req_header); if let Some(variance) = variance { + // Variance is on. This is the primary slot. if !session.cache.cache_vary_lookup(variance, &meta) { - // cache key variance updated, need to lookup again + // This wasn't the desired variant. Updated cache key variance, cause another + // lookup to get the desired variant, which would be in a secondary slot. continue; } - } //else: vary is not in use + } // else: vary is not in use } - // either no variance or the current handler is the variance + // Either no variance, or the current handler targets the correct variant. // hit // TODO: maybe round and/or cache now() @@ -206,6 +210,7 @@ impl HttpProxy { } else { // cache miss if session.cache.is_cache_locked() { + // Another request is filling the cache; try waiting til that's done and retry. let lock_status = session.cache.cache_lock_wait().await; if self.handle_lock_status(session, ctx, lock_status) { continue; diff --git a/pingora-proxy/src/proxy_trait.rs b/pingora-proxy/src/proxy_trait.rs index 80686e8..73eefe8 100644 --- a/pingora-proxy/src/proxy_trait.rs +++ b/pingora-proxy/src/proxy_trait.rs @@ -57,7 +57,7 @@ pub trait ProxyHttp { /// This filter decides if the request is cacheable and what cache backend to use /// - /// The caller can interact with `Session.cache` to enabled caching. + /// The caller can interact with `Session.cache` to enable caching. /// /// By default this filter does nothing which effectively disables caching. // Ideally only session.cache should be modified, TODO: reflect that in this interface @@ -127,7 +127,7 @@ pub trait ProxyHttp { /// Decide how to generate cache vary key from both request and response /// - /// None means no variance is need. + /// None means no variance is needed. fn cache_vary_filter( &self, _meta: &CacheMeta,