caching_ext: move spawn_memcache_writes determination into memcache handler

Summary: The `spawn_memcache_writes` feature used by changesets and the new commit graph should be part of the memcache handler, rather than externally implemented.  Essentially we want the real memcache handler to always spawn writes, however when dealing with mocks or no-ops, there is no need to spawn, and in fact spawning would be harmful by making the test less deterministic.

Differential Revision: D43410854

fbshipit-source-id: 0be62a4b49fcc0bd62aabba1d65d10f6d16e7129

eden/mononoke/changesets/changesets_impl/src/caching.rs

@@ -279,16 +279,6 @@ impl EntityStore<ChangesetEntryWrapper> for CacheRequest<'_> {
     }
 
     caching_ext::impl_singleton_stats!("changesets");
-
-    #[cfg(test)]
-    fn spawn_memcache_writes(&self) -> bool {
-        let (_, mapping) = self;
-
-        match mapping.memcache {
-            MemcacheHandler::Real(_) => true,
-            MemcacheHandler::Mock(..) | MemcacheHandler::Noop => false,
-        }
-    }
 }
 
 #[async_trait]

eden/mononoke/common/rust/caching_ext/src/lib.rs

@@ -131,13 +131,6 @@ pub trait EntityStore<V> {
     ///
     /// Implement this method with `caching_ext::impl_singleton_stats!` macro, instead of by hand
     fn stats(&self) -> &CacheStats;
-
-    /// Whether Memcache writes should run in the background. This is normally the desired behavior
-    /// so this defaults to true, but for tests it's useful to run them synchronously to get
-    /// consistent outcomes.
-    fn spawn_memcache_writes(&self) -> bool {
-        true
-    }
 }
 
 /// Implement this to make it possible to fetch keys via the cache
@@ -352,12 +345,7 @@ async fn fill_caches_by_key<'a, V>(
 
     fill_multiple_cachelib(store.cachelib(), cachelib_keys);
 
-    fill_multiple_memcache(
-        store.memcache(),
-        memcache_keys,
-        store.spawn_memcache_writes(),
-    )
-    .await;
+    fill_multiple_memcache(store.memcache(), memcache_keys).await;
 }
 
 async fn get_multiple_from_memcache<K, V>(
@@ -435,7 +423,6 @@ fn fill_multiple_cachelib<'a, V>(
 async fn fill_multiple_memcache<'a, V: 'a>(
     memcache: &'a MemcacheHandler,
     data: impl IntoIterator<Item = (MemcacheKey, CacheTtl, &'a V)>,
-    spawn: bool,
 ) where
     V: MemcacheEntity,
 {
@@ -465,7 +452,7 @@ async fn fill_multiple_memcache<'a, V: 'a>(
 
     let fut = stream::iter(futs).for_each_concurrent(MEMCACHE_CONCURRENCY, |fut| fut);
 
-    if spawn {
+    if memcache.is_async() {
         tokio::task::spawn(fut);
     } else {
         fut.await;
@@ -536,10 +523,6 @@ mod test {
         }
 
         impl_singleton_stats!("test");
-
-        fn spawn_memcache_writes(&self) -> bool {
-            false
-        }
     }
 
     #[async_trait]

eden/mononoke/common/rust/caching_ext/src/memcache_utils.rs

@@ -37,6 +37,13 @@ impl MemcacheHandler {
         }
     }
 
+    pub fn is_async(&self) -> bool {
+        match self {
+            MemcacheHandler::Real(_) => true,
+            MemcacheHandler::Mock(_) | MemcacheHandler::Noop => false,
+        }
+    }
+
     pub async fn get(&self, key: String) -> Result<Option<Bytes>> {
         match self {
             MemcacheHandler::Real(ref client) => {

eden/mononoke/repo_attributes/commit_graph/caching_commit_graph_storage/src/lib.rs

@@ -181,14 +181,6 @@ impl EntityStore<CachedChangesetEdges> for CacheRequest<'_> {
     }
 
     caching_ext::impl_singleton_stats!("commit_graph");
-
-    #[cfg(test)]
-    fn spawn_memcache_writes(&self) -> bool {
-        match self.caching_storage.memcache {
-            MemcacheHandler::Real(_) => true,
-            MemcacheHandler::Mock(..) | MemcacheHandler::Noop => false,
-        }
-    }
 }
 
 #[async_trait]