diff --git a/eden/mononoke/commit_rewriting/backsyncer/src/tests.rs b/eden/mononoke/commit_rewriting/backsyncer/src/tests.rs index e61b27e8d8..411ef8a296 100644 --- a/eden/mononoke/commit_rewriting/backsyncer/src/tests.rs +++ b/eden/mononoke/commit_rewriting/backsyncer/src/tests.rs @@ -1487,6 +1487,7 @@ async fn preserve_premerge_commit( large_bcs_id: bcs_id, small_repo_id: another_repo_id, small_bcs_id: None, + version_name: Some(CommitSyncConfigVersion("TEST_VERSION_NAME".to_string())), }, ) .compat() diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs index e91c36be06..1d3af39194 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/commit_sync_outcome.rs @@ -230,10 +230,10 @@ pub async fn get_plural_commit_sync_outcome<'a, M: SyncedCommitMapping>( match maybe_wc_equivalence { None => Ok(None), - Some(WorkingCopyEquivalence::NoWorkingCopy) => { + Some(WorkingCopyEquivalence::NoWorkingCopy(_mapping)) => { Ok(Some(PluralCommitSyncOutcome::NotSyncCandidate)) } - Some(WorkingCopyEquivalence::WorkingCopy(cs_id)) => { + Some(WorkingCopyEquivalence::WorkingCopy(cs_id, _)) => { if source_cs_id.0 == cs_id { Ok(Some(PluralCommitSyncOutcome::Preserved)) } else { diff --git a/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs b/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs index 7b2132105c..11fc5136ca 100644 --- a/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs +++ b/eden/mononoke/commit_rewriting/cross_repo_sync/src/lib.rs @@ -1321,6 +1321,8 @@ where .. } => (small_repo, large_repo, false), }; + // TODO(stash): use the real version that was used to remap a commit + let version_name = self.get_current_version(&ctx)?; let source_repoid = source_repo.get_repoid(); let target_repoid = target_repo.get_repoid(); @@ -1333,6 +1335,7 @@ where large_bcs_id: source_bcs_id, small_repo_id: target_repoid, small_bcs_id: Some(target_bcs_id), + version_name: Some(version_name), } } else { EquivalentWorkingCopyEntry { @@ -1340,6 +1343,7 @@ where large_bcs_id: target_bcs_id, small_repo_id: source_repoid, small_bcs_id: Some(source_bcs_id), + version_name: Some(version_name), } } } @@ -1354,6 +1358,7 @@ where large_bcs_id: source_bcs_id, small_repo_id: target_repoid, small_bcs_id: None, + version_name: Some(version_name), } } }; diff --git a/eden/mononoke/commit_rewriting/synced_commit_mapping/schemas/sqlite-synced-commit-mapping.sql b/eden/mononoke/commit_rewriting/synced_commit_mapping/schemas/sqlite-synced-commit-mapping.sql index 8a2c4b28a8..342dae9052 100644 --- a/eden/mononoke/commit_rewriting/synced_commit_mapping/schemas/sqlite-synced-commit-mapping.sql +++ b/eden/mononoke/commit_rewriting/synced_commit_mapping/schemas/sqlite-synced-commit-mapping.sql @@ -21,6 +21,7 @@ CREATE TABLE `synced_working_copy_equivalence` ( `small_bcs_id` binary(32), `large_repo_id` int(11) NOT NULL, `large_bcs_id` binary(32) NOT NULL, + `sync_map_version_name` varchar(255), UNIQUE (`large_repo_id`,`small_repo_id`,`large_bcs_id`) ); diff --git a/eden/mononoke/commit_rewriting/synced_commit_mapping/src/lib.rs b/eden/mononoke/commit_rewriting/synced_commit_mapping/src/lib.rs index 30710dd092..c39702c207 100644 --- a/eden/mononoke/commit_rewriting/synced_commit_mapping/src/lib.rs +++ b/eden/mononoke/commit_rewriting/synced_commit_mapping/src/lib.rs @@ -26,10 +26,14 @@ use thiserror::Error; #[derive(Debug, Eq, Error, PartialEq)] pub enum ErrorKind { - #[error("tried to insert inconsistent small bcs id {expected:?}, while db has {actual:?}")] + #[error( + "tried to insert inconsistent small bcs id {expected_bcs_id:?} version {expected_config_version:?}, while db has {actual_bcs_id:?} version {actual_config_version:?}" + )] InconsistentWorkingCopyEntry { - expected: Option, - actual: Option, + expected_bcs_id: Option, + expected_config_version: Option, + actual_bcs_id: Option, + actual_config_version: Option, }, } @@ -78,15 +82,16 @@ pub struct EquivalentWorkingCopyEntry { pub large_bcs_id: ChangesetId, pub small_repo_id: RepositoryId, pub small_bcs_id: Option, + pub version_name: Option, } #[derive(Debug, PartialEq, Eq)] pub enum WorkingCopyEquivalence { /// There's no matching working copy. It can happen if a pre-big-merge commit from one small /// repo is mapped into another small repo - NoWorkingCopy, - /// ChangesetId of matching working copy. - WorkingCopy(ChangesetId), + NoWorkingCopy(Option), + /// ChangesetId of matching working copy and CommitSyncConfigVersion that was used for mapping + WorkingCopy(ChangesetId, Option), } pub trait SyncedCommitMapping: Send + Sync { @@ -230,17 +235,21 @@ queries! { large_bcs_id: ChangesetId, small_repo_id: RepositoryId, small_bcs_id: Option, + sync_map_version_name: Option, )) { insert_or_ignore, - "{insert_or_ignore} INTO synced_working_copy_equivalence (large_repo_id, large_bcs_id, small_repo_id, small_bcs_id) VALUES {values}" + "{insert_or_ignore} + INTO synced_working_copy_equivalence + (large_repo_id, large_bcs_id, small_repo_id, small_bcs_id, sync_map_version_name) + VALUES {values}" } read SelectWorkingCopyEquivalence( source_repo_id: RepositoryId, bcs_id: ChangesetId, target_repo_id: RepositoryId, - ) -> (RepositoryId, ChangesetId, RepositoryId, Option) { - "SELECT large_repo_id, large_bcs_id, small_repo_id, small_bcs_id + ) -> (RepositoryId, ChangesetId, RepositoryId, Option, Option) { + "SELECT large_repo_id, large_bcs_id, small_repo_id, small_bcs_id, sync_map_version_name FROM synced_working_copy_equivalence WHERE (large_repo_id = {source_repo_id} AND small_repo_id = {target_repo_id} AND large_bcs_id = {bcs_id}) OR (large_repo_id = {target_repo_id} AND small_repo_id = {source_repo_id} AND small_bcs_id = {bcs_id}) @@ -424,12 +433,20 @@ impl SyncedCommitMapping for SqlSyncedCommitMapping { large_bcs_id, small_repo_id, small_bcs_id, + version_name, } = entry; + let version_name_clone = version_name.clone(); let this = self.clone(); InsertWorkingCopyEquivalence::query( &self.write_connection, - &[(&large_repo_id, &large_bcs_id, &small_repo_id, &small_bcs_id)], + &[( + &large_repo_id, + &large_bcs_id, + &small_repo_id, + &small_bcs_id, + &version_name.map(|vn| vn.0), + )], ) .and_then(move |result| { if result.affected_rows() == 1 { @@ -445,15 +462,18 @@ impl SyncedCommitMapping for SqlSyncedCommitMapping { .and_then(move |maybe_equivalent_wc| { if let Some(equivalent_wc) = maybe_equivalent_wc { use WorkingCopyEquivalence::*; - let expected_small_bcs_id = match equivalent_wc { - WorkingCopy(wc) => Some(wc), - NoWorkingCopy => None, + let (expected_bcs_id, expected_version) = match equivalent_wc { + WorkingCopy(wc, mapping) => (Some(wc), mapping), + NoWorkingCopy(mapping) => (None, mapping), }; - - if expected_small_bcs_id != small_bcs_id { + if (expected_bcs_id != small_bcs_id) + || (expected_version != version_name_clone) + { let err = ErrorKind::InconsistentWorkingCopyEntry { - actual: small_bcs_id, - expected: expected_small_bcs_id, + expected_bcs_id, + expected_config_version: expected_version, + actual_bcs_id: small_bcs_id, + actual_config_version: version_name_clone, }; return Err(err.into()); } @@ -499,16 +519,28 @@ impl SyncedCommitMapping for SqlSyncedCommitMapping { .map(move |maybe_row| { match maybe_row { Some(row) => { - let (large_repo_id, large_bcs_id, _small_repo_id, maybe_small_bcs_id) = row; + let ( + large_repo_id, + large_bcs_id, + _small_repo_id, + maybe_small_bcs_id, + maybe_mapping, + ) = row; if target_repo_id == large_repo_id { - Some(WorkingCopyEquivalence::WorkingCopy(large_bcs_id)) + Some(WorkingCopyEquivalence::WorkingCopy( + large_bcs_id, + maybe_mapping.map(CommitSyncConfigVersion), + )) } else { match maybe_small_bcs_id { - Some(small_bcs_id) => { - Some(WorkingCopyEquivalence::WorkingCopy(small_bcs_id)) - } - None => Some(WorkingCopyEquivalence::NoWorkingCopy), + Some(small_bcs_id) => Some(WorkingCopyEquivalence::WorkingCopy( + small_bcs_id, + maybe_mapping.map(CommitSyncConfigVersion), + )), + None => Some(WorkingCopyEquivalence::NoWorkingCopy( + maybe_mapping.map(CommitSyncConfigVersion), + )), } } } @@ -558,30 +590,37 @@ pub fn add_many_in_txn( .collect(); InsertMapping::query_with_transaction(txn, &insert_entries).and_then(move |(txn, _result)| { - let wces: Vec<_> = unwrapped_entries - .into_iter() + // We have to create temp_wce_entries because InsertWorkingCopyEquivalence requires + // and array of references. + let temp_wce_entries: Vec<_> = unwrapped_entries + .iter() .map( - |(large_repo_id, large_bcs_id, small_repo_id, small_bcs_id, _)| { - EquivalentWorkingCopyEntry { - large_repo_id, - large_bcs_id, - small_repo_id, - small_bcs_id: Some(small_bcs_id), - } + |(large_repo_id, large_bcs_id, small_repo_id, small_bcs_id, version_name)| { + ( + *large_repo_id, + *large_bcs_id, + *small_repo_id, + Some(*small_bcs_id), + version_name.clone(), + ) }, ) .collect(); - let wce_entries: Vec<_> = wces + let wce_entries: Vec<_> = temp_wce_entries .iter() - .map(|entry| { - ( - &entry.large_repo_id, - &entry.large_bcs_id, - &entry.small_repo_id, - &entry.small_bcs_id, - ) - }) + .map( + |(large_repo_id, large_bcs_id, small_repo_id, small_bcs_id, version_name)| { + ( + large_repo_id, + large_bcs_id, + small_repo_id, + small_bcs_id, + version_name, + ) + }, + ) .collect(); + InsertWorkingCopyEquivalence::query_with_transaction(txn, &wce_entries) .map(|(txn, result)| (txn, result.affected_rows())) }) diff --git a/eden/mononoke/commit_rewriting/synced_commit_mapping/test/main.rs b/eden/mononoke/commit_rewriting/synced_commit_mapping/test/main.rs index f2f4e640f1..9e1ed64f9e 100644 --- a/eden/mononoke/commit_rewriting/synced_commit_mapping/test/main.rs +++ b/eden/mononoke/commit_rewriting/synced_commit_mapping/test/main.rs @@ -23,13 +23,14 @@ use synced_commit_mapping::{ }; async fn add_and_get(fb: FacebookInit, mapping: M) { + let version_name = CommitSyncConfigVersion("TEST_VERSION_NAME".to_string()); let ctx = CoreContext::test_mock(fb); let entry = SyncedCommitMappingEntry::new( REPO_ZERO, bonsai::ONES_CSID, REPO_ONE, bonsai::TWOS_CSID, - Some(CommitSyncConfigVersion("TEST_VERSION_NAME".to_string())), + Some(version_name.clone()), ); assert_eq!( true, @@ -56,7 +57,10 @@ async fn add_and_get(fb: FacebookInit, mapping: M) { assert_eq!( res, - Some(WorkingCopyEquivalence::WorkingCopy(bonsai::TWOS_CSID)) + Some(WorkingCopyEquivalence::WorkingCopy( + bonsai::TWOS_CSID, + Some(version_name.clone()), + )) ); // insert again @@ -84,7 +88,10 @@ async fn add_and_get(fb: FacebookInit, mapping: M) { assert_eq!( res, - Some(WorkingCopyEquivalence::WorkingCopy(bonsai::FOURS_CSID)) + Some(WorkingCopyEquivalence::WorkingCopy( + bonsai::FOURS_CSID, + Some(version_name.clone()), + )) ); let result = mapping @@ -117,6 +124,7 @@ async fn missing(fb: FacebookInit, mapping: M) { async fn equivalent_working_copy(fb: FacebookInit, mapping: M) { let ctx = CoreContext::test_mock(fb); + let version_name = CommitSyncConfigVersion("TEST_VERSION_NAME".to_string()); let result = mapping .get_equivalent_working_copy(ctx.clone(), REPO_ONE, bonsai::TWOS_CSID, REPO_ZERO) .compat() @@ -129,6 +137,7 @@ async fn equivalent_working_copy(fb: FacebookInit, mappi large_bcs_id: bonsai::ONES_CSID, small_repo_id: REPO_ONE, small_bcs_id: Some(bonsai::TWOS_CSID), + version_name: Some(version_name.clone()), }; let result = mapping .insert_equivalent_working_copy(ctx.clone(), entry.clone()) @@ -152,7 +161,10 @@ async fn equivalent_working_copy(fb: FacebookInit, mappi assert_eq!( res, - Some(WorkingCopyEquivalence::WorkingCopy(bonsai::TWOS_CSID)) + Some(WorkingCopyEquivalence::WorkingCopy( + bonsai::TWOS_CSID, + Some(version_name) + )) ); let null_entry = EquivalentWorkingCopyEntry { @@ -160,6 +172,7 @@ async fn equivalent_working_copy(fb: FacebookInit, mappi large_bcs_id: bonsai::THREES_CSID, small_repo_id: REPO_ONE, small_bcs_id: None, + version_name: None, }; let result = mapping @@ -175,13 +188,14 @@ async fn equivalent_working_copy(fb: FacebookInit, mappi .await .expect("get equivalent wc failed, should succeed"); - assert_eq!(res, Some(WorkingCopyEquivalence::NoWorkingCopy)); + assert_eq!(res, Some(WorkingCopyEquivalence::NoWorkingCopy(None))); let should_fail = EquivalentWorkingCopyEntry { large_repo_id: REPO_ZERO, large_bcs_id: bonsai::THREES_CSID, small_repo_id: REPO_ONE, small_bcs_id: Some(bonsai::TWOS_CSID), + version_name: None, }; assert!( mapping