megarepo_api: remove mutable rename creation

Summary: Use of mutable renames to track the moves caused by megarepo-api move commits has turned out not to scale well. Let's remove them and take a different approach in the future. Reviewed By: andreacampi Differential Revision: D67759504 fbshipit-source-id: b118860e48e530cefd813139888fe80240c09484
facebook · Jan 3, 2025 · dbd8fc7 · dbd8fc7
1 parent 94750d9
commit dbd8fc7
Show file tree

Hide file tree

Showing 13 changed files with 47 additions and 316 deletions.
diff --git a/eden/mononoke/megarepo_api/Cargo.toml b/eden/mononoke/megarepo_api/Cargo.toml
@@ -37,7 +37,6 @@ metaconfig_types = { version = "0.1.0", path = "../metaconfig/types" }
 mononoke_api = { version = "0.1.0", path = "../mononoke_api" }
 mononoke_app = { version = "0.1.0", path = "../cmdlib/mononoke_app" }
 mononoke_types = { version = "0.1.0", path = "../mononoke_types" }
-mutable_renames = { version = "0.1.0", path = "../mutable_renames" }
 parking_lot = { version = "0.12.1", features = ["send_guard"] }
 repo_authorization = { version = "0.1.0", path = "../repo_authorization" }
 repo_blobstore = { version = "0.1.0", path = "../blobrepo/repo_blobstore" }
@@ -47,7 +46,6 @@ repo_identity = { version = "0.1.0", path = "../repo_attributes/repo_identity" }
 serde_json = { version = "1.0.132", features = ["float_roundtrip", "unbounded_depth"] }
 slog = { version = "2.7", features = ["max_level_trace", "nested-values"] }
 sorted_vector_map = { version = "0.2.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" }
-unodes = { version = "0.1.0", path = "../derived_data/unodes" }
 
 [dev-dependencies]
 fbinit = { version = "0.2.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "main" }

diff --git a/eden/mononoke/megarepo_api/src/add_branching_sync_target_test.rs b/eden/mononoke/megarepo_api/src/add_branching_sync_target_test.rs
@@ -52,8 +52,7 @@ async fn test_add_branching_sync_target_success(fb: FacebookInit) -> Result<(),
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -141,8 +140,7 @@ async fn test_add_branching_sync_target_wrong_branch(fb: FacebookInit) -> Result
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -184,8 +182,7 @@ async fn test_add_branching_sync_target_wrong_branch(fb: FacebookInit) -> Result
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;

diff --git a/eden/mononoke/megarepo_api/src/add_sync_target.rs b/eden/mononoke/megarepo_api/src/add_sync_target.rs
@@ -22,7 +22,6 @@ use mononoke_api::Mononoke;
 use mononoke_api::MononokeRepo;
 use mononoke_api::RepoContext;
 use mononoke_types::ChangesetId;
-use mutable_renames::MutableRenames;
 
 use crate::common::derive_all_types;
 use crate::common::MegarepoOp;
@@ -48,7 +47,6 @@ use crate::common::MegarepoOp;
 pub struct AddSyncTarget<'a, R> {
     pub megarepo_configs: &'a Arc<dyn MononokeMegarepoConfigs>,
     pub mononoke: &'a Arc<Mononoke<R>>,
-    pub mutable_renames: &'a Arc<MutableRenames>,
 }
 
 impl<'a, R> MegarepoOp<R> for AddSyncTarget<'a, R> {
@@ -61,12 +59,10 @@ impl<'a, R: MononokeRepo> AddSyncTarget<'a, R> {
     pub fn new(
         megarepo_configs: &'a Arc<dyn MononokeMegarepoConfigs>,
         mononoke: &'a Arc<Mononoke<R>>,
-        mutable_renames: &'a Arc<MutableRenames>,
     ) -> Self {
         Self {
             megarepo_configs,
             mononoke,
-            mutable_renames,
         }
     }
 
@@ -106,7 +102,6 @@ impl<'a, R: MononokeRepo> AddSyncTarget<'a, R> {
                 repo.repo(),
                 &sync_target_config.sources,
                 &changesets_to_merge,
-                self.mutable_renames,
             )
             .await?;
         scuba.log_with_msg("Created move commits", None);

diff --git a/eden/mononoke/megarepo_api/src/add_sync_target_test.rs b/eden/mononoke/megarepo_api/src/add_sync_target_test.rs
@@ -75,8 +75,7 @@ async fn test_add_sync_target_simple(fb: FacebookInit) -> Result<(), Error> {
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -141,12 +140,8 @@ async fn test_add_sync_target_simple(fb: FacebookInit) -> Result<(), Error> {
         .set_to(cs_id)
         .await?;
 
-    let sync_changeset = SyncChangeset::new(
-        &configs_storage,
-        &test.mononoke,
-        &test.megarepo_mapping,
-        &test.mutable_renames,
-    );
+    let sync_changeset =
+        SyncChangeset::new(&configs_storage, &test.mononoke, &test.megarepo_mapping);
 
     sync_changeset
         .sync(&ctx, cs_id, &first_source_name, &target, target_cs_id)
@@ -216,8 +211,7 @@ async fn test_add_sync_target_with_linkfiles(fb: FacebookInit) -> Result<(), Err
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -307,8 +301,7 @@ async fn test_add_sync_target_invalid_same_prefix(fb: FacebookInit) -> Result<()
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -374,8 +367,7 @@ async fn test_add_sync_target_same_file_different_prefix(fb: FacebookInit) -> Re
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -474,8 +466,7 @@ async fn test_add_sync_target_invalid_linkfiles(fb: FacebookInit) -> Result<(),
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -532,8 +523,7 @@ async fn test_add_sync_target_invalid_hash_to_merge(fb: FacebookInit) -> Result<
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -611,8 +601,7 @@ async fn test_add_sync_target_merge_three_sources(fb: FacebookInit) -> Result<()
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -717,8 +706,7 @@ async fn test_add_sync_target_repeat_same_request(fb: FacebookInit) -> Result<()
 
     let configs_storage: Arc<dyn MononokeMegarepoConfigs> = Arc::new(test.configs_storage.clone());
 
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let repo = add_sync_target
         .find_repo_by_id(&ctx, target.repo_id)
         .await?;
@@ -742,8 +730,7 @@ async fn test_add_sync_target_repeat_same_request(fb: FacebookInit) -> Result<()
 
     // Now repeat the same request again (as if client retries a reqeust that has already
     // succeeded). We should get the same result as the first time.
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let second_result = add_sync_target
         .run(
             &ctx,
@@ -759,8 +746,7 @@ async fn test_add_sync_target_repeat_same_request(fb: FacebookInit) -> Result<()
     assert_eq!(first_result, second_result);
 
     // Now modify the request - it should fail
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     assert!(
         add_sync_target
             .run(
@@ -777,8 +763,7 @@ async fn test_add_sync_target_repeat_same_request(fb: FacebookInit) -> Result<()
 
     // Now send different config with the same name - should fail
     sync_target_config.sources = vec![];
-    let add_sync_target =
-        AddSyncTarget::new(&configs_storage, &test.mononoke, &test.mutable_renames);
+    let add_sync_target = AddSyncTarget::new(&configs_storage, &test.mononoke);
     let res = add_sync_target
         .run(
             &ctx,

diff --git a/eden/mononoke/megarepo_api/src/change_target_config.rs b/eden/mononoke/megarepo_api/src/change_target_config.rs
@@ -26,7 +26,6 @@ use mononoke_api::Mononoke;
 use mononoke_api::MononokeRepo;
 use mononoke_api::RepoContext;
 use mononoke_types::ChangesetId;
-use mutable_renames::MutableRenames;
 
 use crate::common::derive_all_types;
 use crate::common::find_target_bookmark_and_value;
@@ -148,7 +147,6 @@ fn diff_configs(
 pub struct ChangeTargetConfig<'a, R> {
     pub megarepo_configs: &'a Arc<dyn MononokeMegarepoConfigs>,
     pub mononoke: &'a Arc<Mononoke<R>>,
-    pub mutable_renames: &'a Arc<MutableRenames>,
 }
 
 impl<'a, R> MegarepoOp<R> for ChangeTargetConfig<'a, R> {
@@ -161,12 +159,10 @@ impl<'a, R: MononokeRepo> ChangeTargetConfig<'a, R> {
     pub fn new(
         megarepo_configs: &'a Arc<dyn MononokeMegarepoConfigs>,
         mononoke: &'a Arc<Mononoke<R>>,
-        mutable_renames: &'a Arc<MutableRenames>,
     ) -> Self {
         Self {
             megarepo_configs,
             mononoke,
-            mutable_renames,
         }
     }
 
@@ -254,7 +250,6 @@ impl<'a, R: MononokeRepo> ChangeTargetConfig<'a, R> {
                 &changesets_to_merge,
                 &new_config,
                 message.clone(),
-                self.mutable_renames,
                 target.bookmark.to_owned(),
             )
             .await?;
@@ -319,7 +314,6 @@ impl<'a, R: MononokeRepo> ChangeTargetConfig<'a, R> {
         changesets_to_merge: &BTreeMap<SourceName, ChangesetId>,
         sync_target_config: &SyncTargetConfig,
         message: Option<String>,
-        mutable_renames: &Arc<MutableRenames>,
         bookmark: String,
     ) -> Result<Option<ChangesetId>, MegarepoError> {
         if diff.added.is_empty() {
@@ -332,13 +326,7 @@ impl<'a, R: MononokeRepo> ChangeTargetConfig<'a, R> {
             .map(|(source, _cs_id)| source.clone())
             .collect();
         let moved_commits = self
-            .create_move_commits(
-                ctx,
-                repo.repo(),
-                &sources_to_add,
-                changesets_to_merge,
-                mutable_renames,
-            )
+            .create_move_commits(ctx, repo.repo(), &sources_to_add, changesets_to_merge)
             .await?;
 
         if moved_commits.len() == 1 {