scmstore: fix propagation of CAS aux data to EdenFS

Summary:
Fix DynTree trait impl to support propagating AUX data received from CAS tree entries. Make use of LazyTree::children_aux_data, which already abstracts across the different sources of tree data.

When EdenFS fetches trees via Sapling, it will also make use of aux data for the tree entries when available. genevievehelsel discovered the aux data was not making it when Sapling was configured to fetch data from the CAS backend. This hurts performance because eden's caches are not warmed with aux data. The CAS trees _should_ be warm in the local cache, so fetching aux data subsequently should not trigger a remote fetch.

Reviewed By: liubov-dmitrieva

Differential Revision: D71522121

fbshipit-source-id: b58bb7920941f95f359f75dd2976b185aac40133

Author: muirdm

eden/scm/lib/revisionstore/src/scmstore/tree.rs

@@ -30,7 +30,6 @@ use clientinfo::get_client_request_info_thread_local;
 use clientinfo::set_client_request_info_thread_local;
 use edenapi_types::FileAuxData;
 use edenapi_types::TreeAuxData;
-use edenapi_types::TreeChildEntry;
 use fetch::FetchState;
 use flume::bounded;
 use flume::unbounded;
@@ -44,6 +43,7 @@ use storemodel::InsertOpts;
 use storemodel::KeyStore;
 use storemodel::SerializationFormat;
 use storemodel::TreeEntry;
+use types::AuxData;
 
 use crate::datastore::HgIdDataStore;
 use crate::historystore::HistoryStore;
@@ -833,58 +833,29 @@ impl TreeEntry for ScmStoreTreeEntry {
     }
 
     fn file_aux_iter(&self) -> anyhow::Result<BoxIterator<anyhow::Result<(HgId, FileAuxData)>>> {
-        let maybe_iter = (move || -> Option<BoxIterator<anyhow::Result<(HgId, FileAuxData)>>> {
-            let entry = match &self.tree {
-                LazyTree::SaplingRemoteApi(entry) => entry.clone(),
-                _ => return None,
-            };
-            let children = entry.children?;
-            let iter = children.into_iter().filter_map(move |child| {
-                let child = child.as_ref().ok()?;
-                let file_entry = match child {
-                    TreeChildEntry::File(v) => v,
-                    _ => return None,
-                };
-                Some(Ok((
-                    file_entry.key.hgid,
-                    file_entry.file_metadata.clone()?.into(),
-                )))
-            });
-            Some(Box::new(iter))
-        })();
-        Ok(maybe_iter.unwrap_or_else(|| Box::new(std::iter::empty())))
+        Ok(Box::new(
+            self.tree
+                .children_aux_data()
+                .into_iter()
+                .filter_map(|(hgid, aux)| match aux {
+                    AuxData::File(file_aux_data) => Some(Ok((hgid, file_aux_data))),
+                    AuxData::Tree(_) => None,
+                }),
+        ))
     }
 
     fn tree_aux_data_iter(
         &self,
     ) -> anyhow::Result<BoxIterator<anyhow::Result<(HgId, TreeAuxData)>>> {
-        let maybe_iter = (|| -> Option<BoxIterator<anyhow::Result<(HgId, TreeAuxData)>>> {
-            let entry = match &self.tree {
-                LazyTree::SaplingRemoteApi(entry) => entry.clone(),
-                // TODO: We should also support fetching tree metadata from local cache
-                _ => return None,
-            };
-            let children = entry.children?;
-            let iter = children.into_iter().filter_map(|child| {
-                let child = child.as_ref().ok()?;
-                let directory_entry = match child {
-                    TreeChildEntry::Directory(v) => v,
-                    _ => return None,
-                };
-                let tree_aux_data = directory_entry
-                    .tree_aux_data
-                    .ok_or_else(|| {
-                        anyhow::anyhow!(format!(
-                            "tree aux data is missing for key: {}",
-                            directory_entry.key
-                        ))
-                    })
-                    .ok()?;
-                Some(Ok((directory_entry.key.hgid, tree_aux_data)))
-            });
-            Some(Box::new(iter))
-        })();
-        Ok(maybe_iter.unwrap_or_else(|| Box::new(std::iter::empty())))
+        Ok(Box::new(
+            self.tree
+                .children_aux_data()
+                .into_iter()
+                .filter_map(|(hgid, aux)| match aux {
+                    AuxData::File(_) => None,
+                    AuxData::Tree(tree_aux_data) => Some(Ok((hgid, tree_aux_data))),
+                }),
+        ))
     }
 
     /// Get the directory aux data of the tree.

eden/scm/tests/test-scmstore-storemodel-aux-data.t

@@ -1,5 +1,10 @@
+#testcases cas no-cas
 
+#if cas
   $ setconfig scmstore.cas-mode=on
+#else
+  $ setconfig scmstore.cas-mode=off
+#endif
 
   $ setconfig push.edenapi=true
   $ setconfig scmstore.tree-metadata-mode=always
@@ -21,9 +26,10 @@ First fetch aux data for root dir (needed to for subsequent fetch).
   $ hg debugscmstore --mode tree -r $A "" >/dev/null
 
 --store-model uses the storemodel trait (which is what EdenFS uses)
-FIXME: missing aux data
   $ hg debugscmstore --mode tree -r $A "dir" --store-model
   Tree 'dir' entries
     (PathComponentBuf("bar"), HgId("a324b8bf63f7d56de9d36f8747e3b68a72a4d968"), File(Regular))
     (PathComponentBuf("foo"), HgId("49d8cbb15ce257920447006b46978b7af980a979"), File(Regular))
   Tree 'dir' file aux
+    (HgId("a324b8bf63f7d56de9d36f8747e3b68a72a4d968"), FileAuxData { total_size: 3, sha1: Sha1("62cdb7020ff920e5aa642c3d4066950dd1f01f4d"), blake3: Blake3("0f6cb4c2a4c391b24fdbb4023e5d8c9ad7fbadaecfdc69cc801cf9d08dffd32a"), file_header_metadata: Some(b"") })
+    (HgId("49d8cbb15ce257920447006b46978b7af980a979"), FileAuxData { total_size: 3, sha1: Sha1("0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33"), blake3: Blake3("29b9cb87c1ac3fd85f385e97eed4ba6b0bc64435cc1985815d2bef6cde472233"), file_header_metadata: Some(b"") })