repo api: Replace prepare_derived_data with derive_bulk, generali…

…zing it

Summary:
With the introduction of the `BulkDerivation` trait on `derived_data_manager` in the previous diff,
`mononoke_api` doesn't need to depend on and enumerate every single derived data type any more to offer the functionality it wants to offer.

Rename `prepare_derived_data` to `derive_bulk` to allow for more transparent abstractions between the derived data world and the mononoke api world.

Reviewed By: markbt

Differential Revision: D54264050

fbshipit-source-id: d853419b38ab07212b2ccf48facc7288c19cbc6d

eden/mononoke/mononoke_api/Cargo.toml

@@ -27,6 +27,7 @@ bookmarks_cache = { version = "0.1.0", path = "../bookmarks/bookmarks_cache" }
 bookmarks_movement = { version = "0.1.0", path = "../bookmarks/bookmarks_movement" }
 borrowed = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" }
 bounded_traversal = { version = "0.1.0", path = "../common/bounded_traversal" }
+bulk_derivation = { version = "0.1.0", path = "../derived_data/bulk_derivation" }
 bytes = { version = "1.1", features = ["serde"] }
 cacheblob = { version = "0.1.0", path = "../blobstore/cacheblob" }
 changeset_fetcher = { version = "0.1.0", path = "../blobrepo/changeset_fetcher" }

eden/mononoke/mononoke_api/TARGETS

@@ -89,6 +89,7 @@ rust_library(
         "//eden/mononoke/common/sql_construct:sql_construct",
         "//eden/mononoke/derived_data:basename_suffix_skeleton_manifest_v3",
         "//eden/mononoke/derived_data:blame",
+        "//eden/mononoke/derived_data:bulk_derivation",
         "//eden/mononoke/derived_data:changeset_info",
         "//eden/mononoke/derived_data:deleted_manifest",
         "//eden/mononoke/derived_data:derived_data",

eden/mononoke/mononoke_api/src/repo.rs

@@ -11,6 +11,7 @@ use std::fmt;
 use std::hash::Hash;
 use std::hash::Hasher;
 use std::sync::Arc;
+use std::time::Duration;
 use std::time::SystemTime;
 use std::time::UNIX_EPOCH;
 
@@ -48,6 +49,7 @@ use bookmarks::BookmarksRef;
 pub use bookmarks::Freshness as BookmarkFreshness;
 use bookmarks::Freshness;
 use bookmarks_cache::BookmarksCache;
+use bulk_derivation::BulkDerivation;
 use bytes::Bytes;
 use cacheblob::InProcessLease;
 use cacheblob::LeaseOps;
@@ -83,7 +85,6 @@ use filestore::FetchKey;
 use filestore::FilestoreConfig;
 use filestore::FilestoreConfigRef;
 pub use filestore::StoreRequest;
-use fsnodes::RootFsnodeId;
 use futures::compat::Stream01CompatExt;
 use futures::stream;
 use futures::stream::Stream;
@@ -150,7 +151,6 @@ use segmented_changelog::DisabledSegmentedChangelog;
 use segmented_changelog::Location;
 use segmented_changelog::SegmentedChangelog;
 use segmented_changelog::SegmentedChangelogRef;
-use skeleton_manifest::RootSkeletonManifestId;
 use slog::debug;
 use slog::error;
 use sql_construct::SqlConstruct;
@@ -1840,36 +1840,22 @@ impl RepoContext {
         Ok(pull_data)
     }
 
-    pub async fn prepare_derived_data(
+    pub async fn derive_bulk(
         &self,
-        derivable_type: DerivableType,
+        ctx: &CoreContext,
         csids: Vec<ChangesetId>,
-    ) -> Result<(), MononokeError> {
-        // Simple initial implementation: does not support types with
-        // dependencies, and only supports a single type at a time.
-        match derivable_type {
-            DerivableType::Fsnodes => {
-                self.repo
-                    .repo_derived_data()
-                    .manager()
-                    .derive_exactly_batch::<RootFsnodeId>(self.ctx(), csids, None)
-                    .await?;
-            }
-            DerivableType::SkeletonManifests => {
-                self.repo
-                    .repo_derived_data()
-                    .manager()
-                    .derive_exactly_batch::<RootSkeletonManifestId>(self.ctx(), csids, None)
-                    .await?;
-            }
-            _ => {
-                return Err(MononokeError::InvalidRequest(format!(
-                    "Unsupported derived data type for preparation: {}",
-                    derivable_type
-                )));
-            }
-        }
-        Ok(())
+        derivable_types: &[DerivableType],
+    ) -> Result<Duration, MononokeError> {
+        // We don't need to expose rederivation to users of the repo api
+        // That's a lower level concept that clients like the derived data backfiller
+        // can get straight from the derived data manager
+        let rederivation = None;
+        Ok(self
+            .repo
+            .repo_derived_data()
+            .manager()
+            .derive_bulk(ctx, csids, rederivation, derivable_types)
+            .await?)
     }
 }
 

eden/mononoke/scs_server/src/errors.rs

@@ -8,6 +8,7 @@
 use std::backtrace::BacktraceStatus;
 use std::error::Error as StdError;
 
+use derived_data_manager::DerivationError;
 use git_types::GitError;
 use megarepo_error::MegarepoError;
 use mononoke_api::MononokeError;
@@ -199,6 +200,13 @@ impl From<MononokeError> for ServiceError {
     }
 }
 
+impl From<DerivationError> for ServiceError {
+    fn from(e: DerivationError) -> Self {
+        let mononoke_error: MononokeError = e.into();
+        mononoke_error.into()
+    }
+}
+
 macro_rules! impl_into_thrift_error {
     ($t:ty) => {
         impl From<ServiceError> for $t {

eden/mononoke/scs_server/src/methods/repo.rs

@@ -462,7 +462,7 @@ impl SourceControlServiceImpl {
         params: thrift::RepoCreateStackParams,
     ) -> Result<thrift::RepoCreateStackResponse, errors::ServiceError> {
         let repo = self
-            .repo_for_service(ctx, &repo, params.service_identity.clone())
+            .repo_for_service(ctx.clone(), &repo, params.service_identity.clone())
             .await?;
         let repo = &repo;
 
@@ -489,15 +489,14 @@ impl SourceControlServiceImpl {
             .create_changeset_stack(stack_parents, info_stack, changes_stack, bubble)
             .await?;
 
-        // Prepare derived data if we were asked to.  Simple implementation
-        // that doesn't support parallel derivation or dependencies between types.
+        // Prepare derived data if we were asked to.
         if let Some(prepare_types) = &params.prepare_derived_data_types {
             let csids = stack.iter().map(|c| c.id()).collect::<Vec<_>>();
-            for derived_data_type in prepare_types {
-                let derivable_type = DerivableType::from_request(derived_data_type)?;
-                repo.prepare_derived_data(derivable_type, csids.clone())
-                    .await?;
-            }
+            let derived_data_types = prepare_types
+                .iter()
+                .map(DerivableType::from_request)
+                .collect::<Result<Vec<_>, _>>()?;
+            repo.derive_bulk(&ctx, csids, &derived_data_types).await?;
         }
 
         // If you ask for a git identity back, then we'll assume that you supplied one to us
@@ -752,7 +751,7 @@ impl SourceControlServiceImpl {
         repo: thrift::RepoSpecifier,
         params: thrift::RepoPrepareCommitsParams,
     ) -> Result<thrift::RepoPrepareCommitsResponse, errors::ServiceError> {
-        let repo = self.repo(ctx, &repo).await?;
+        let repo = self.repo(ctx.clone(), &repo).await?;
         // Convert thrift commit ids to bonsai changeset ids
         let changesets = try_join_all(
             params
@@ -764,16 +763,16 @@ impl SourceControlServiceImpl {
                 .map(|specifier| repo.changeset(specifier)),
         )
         .await?;
-        let cs_ids = std::iter::zip(params.commits, changesets)
+        let csids = std::iter::zip(params.commits, changesets)
             .map(|(commit, cs)| {
                 cs.map(|cs| cs.id())
                     .ok_or_else(|| errors::commit_not_found(commit.to_string()))
             })
             .collect::<Result<Vec<_>, _>>()?;
 
         // Derive data of the requested type for the batch of desired commits
-        let derivable_type = DerivableType::from_request(&params.derived_data_type)?;
-        repo.prepare_derived_data(derivable_type, cs_ids).await?;
+        let derived_data_type = DerivableType::from_request(&params.derived_data_type)?;
+        repo.derive_bulk(&ctx, csids, &[derived_data_type]).await?;
 
         Ok(thrift::RepoPrepareCommitsResponse {
             ..Default::default()