Skip to content

Commit f1ef619

Browse files
Viet Hung Nguyenfacebook-github-bot
authored andcommitted
mononoke/repo_import: add phabricator lag checker
Summary: Once we start moving the bookmark across the imported commits (D22598159 (c5e880c)), we need to check dependent systems to avoid overloading them when parsing the commits. In this diff we added the functionality to check Phabricator. We use an external service (jf graphql - find discussion here: https://fburl.com/nr1f19gs) to fetch commits from Phabricator. Each commit id starts with "r", followed by a call sign (e.g FBS for fbsource) and the commit hash (https://fburl.com/qa/9pf0vtkk). If we try to fetch an invalid commit id (e.g not having a call sign), we should receive an error. Otherwise, we should receive a JSON. An imported commit should have the following query result: https://fburl.com/graphiql/20txxvsn - nodes has one result with the imported field true. If the commit hasn't been recognised by Phabricator yet, the nodes array will be empty. If the commit has been recognised, but not yet parsed, the imported field will be false. If we haven't parsed the batch, we will try to check Phabricator again after sleeping for a couple of seconds. If it has parsed the batch of commits, we move the bookmark to the next batch. Reviewed By: krallin Differential Revision: D22762800 fbshipit-source-id: 5c02262923524793f364743e3e1b3f46c921db8d
1 parent 22f90df commit f1ef619

File tree

3 files changed

+181
-7
lines changed

3 files changed

+181
-7
lines changed

eden/mononoke/repo_import/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ include = ["src/**/*.rs"]
88

99
[dependencies]
1010
blobrepo = { path = "../blobrepo" }
11+
blobrepo_hg = { path = "../blobrepo/blobrepo_hg" }
1112
bookmarks = { path = "../bookmarks" }
1213
cmdlib = { path = "../cmdlib" }
1314
context = { path = "../server/context" }
@@ -22,7 +23,10 @@ fbinit = { git = "https://github.com/facebookexperimental/rust-shed.git", branch
2223
anyhow = "1.0"
2324
clap = "2.33"
2425
futures = { version = "0.3.5", features = ["async-await", "compat"] }
26+
serde = { version = "1.0", features = ["derive", "rc"] }
27+
serde_json = "1.0"
2528
slog = { version = "2.5", features = ["max_level_debug"] }
29+
tokio = { version = "=0.2.13", features = ["full"] }
2630

2731
[dev-dependencies]
2832
blobrepo_factory = { path = "../blobrepo/factory" }

eden/mononoke/repo_import/src/main.rs

Lines changed: 176 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#![type_length_limit = "4522397"]
99
use anyhow::{format_err, Error};
1010
use blobrepo::{save_bonsai_changesets, BlobRepo};
11+
use blobrepo_hg::BlobRepoHg;
1112
use bookmarks::{BookmarkName, BookmarkUpdateReason};
1213
use clap::Arg;
1314
use cmdlib::args;
@@ -22,19 +23,55 @@ use futures::{
2223
stream::{self, StreamExt, TryStreamExt},
2324
};
2425
use import_tools::{GitimportPreferences, GitimportTarget};
25-
use mercurial_types::MPath;
26+
use mercurial_types::{HgChangesetId, MPath};
2627
use mononoke_types::{BonsaiChangeset, ChangesetId};
2728
use movers::DefaultAction;
29+
use serde::{Deserialize, Serialize};
30+
use serde_json;
2831
use slog::info;
2932
use std::collections::HashMap;
3033
use std::num::NonZeroUsize;
3134
use std::path::Path;
35+
use tokio::{process, time};
3236
use topo_sort::sort_topological;
3337

3438
const ARG_GIT_REPOSITORY_PATH: &str = "git-repository-path";
3539
const ARG_DEST_PATH: &str = "dest-path";
3640
const ARG_BATCH_SIZE: &str = "batch-size";
3741
const ARG_BOOKMARK_SUFFIX: &str = "bookmark-suffix";
42+
const ARG_CALL_SIGN: &str = "call-sign";
43+
const ARG_PHAB_CHECK_DISABLED: &str = "disable-phabricator-check";
44+
const ARG_X_REPO_CHECK_DISABLED: &str = "disable-x-repo-check";
45+
const ARG_HG_SYNC_CHECK_DISABLED: &str = "disable-hg-sync-check";
46+
const ARG_SLEEP_TIME: &str = "sleep-time";
47+
48+
#[derive(Deserialize, Clone, Debug)]
49+
struct GraphqlQueryObj {
50+
differential_commit_query: Vec<GraphqlCommitQueryObj>,
51+
}
52+
#[derive(Deserialize, Clone, Debug)]
53+
struct GraphqlCommitQueryObj {
54+
results: GraphqlResultsObj,
55+
}
56+
#[derive(Deserialize, Clone, Debug)]
57+
struct GraphqlResultsObj {
58+
nodes: Vec<GraphqlImportedObj>,
59+
}
60+
#[derive(Deserialize, Clone, Debug)]
61+
struct GraphqlImportedObj {
62+
imported: bool,
63+
}
64+
#[derive(Debug, Serialize)]
65+
struct GraphqlInputVariables {
66+
commit: String,
67+
}
68+
#[derive(Debug)]
69+
struct CheckerFlags<'a> {
70+
phab_check_disabled: bool,
71+
x_repo_check_disabled: bool,
72+
hg_sync_check_disabled: bool,
73+
call_sign: Option<&'a str>,
74+
}
3875

3976
async fn rewrite_file_paths(
4077
ctx: &CoreContext,
@@ -117,6 +154,8 @@ async fn move_bookmark(
117154
shifted_bcs: &[BonsaiChangeset],
118155
batch_size: usize,
119156
bookmark_suffix: &str,
157+
checker_flags: &CheckerFlags<'_>,
158+
sleep_time: u64,
120159
) -> Result<(), Error> {
121160
if shifted_bcs.is_empty() {
122161
return Err(format_err!("There is no bonsai changeset present"));
@@ -162,11 +201,75 @@ async fn move_bookmark(
162201
ctx.logger(),
163202
"Set bookmark {:?} to point to {:?}", bookmark, curr_csid
164203
);
204+
205+
// if a check is disabled, we have already passed the check
206+
let mut passed_phab_check = checker_flags.phab_check_disabled;
207+
let mut _passed_x_repo_check = checker_flags.x_repo_check_disabled;
208+
let mut _passed_hg_sync_check = checker_flags.hg_sync_check_disabled;
209+
let hg_csid = repo
210+
.get_hg_from_bonsai_changeset(ctx.clone(), curr_csid)
211+
.compat()
212+
.await?;
213+
while !passed_phab_check {
214+
let call_sign = checker_flags.call_sign.as_ref().unwrap();
215+
passed_phab_check = phabricator_commit_check(&call_sign, &hg_csid).await?;
216+
if !passed_phab_check {
217+
info!(
218+
ctx.logger(),
219+
"Phabricator hasn't parsed commit: {:?}", hg_csid
220+
);
221+
time::delay_for(time::Duration::from_secs(sleep_time)).await;
222+
}
223+
}
165224
old_csid = curr_csid;
166225
}
167226
Ok(())
168227
}
169228

229+
async fn phabricator_commit_check(call_sign: &str, hg_csid: &HgChangesetId) -> Result<bool, Error> {
230+
let commit_id = format!("r{}{}", call_sign, hg_csid);
231+
let query = "query($commit: String!) {
232+
differential_commit_query(query_params:{commits:[$commit]}) {
233+
results {
234+
nodes {
235+
imported
236+
}
237+
}
238+
}
239+
}";
240+
let variables = serde_json::to_string(&GraphqlInputVariables { commit: commit_id }).unwrap();
241+
let output = process::Command::new("jf")
242+
.arg("graphql")
243+
.arg("--query")
244+
.arg(query)
245+
.arg("--variables")
246+
.arg(variables)
247+
.output()
248+
.await?;
249+
if !output.status.success() {
250+
let e = format_err!(
251+
"Failed to fetch graphql commit: {}",
252+
String::from_utf8_lossy(&output.stderr)
253+
);
254+
return Err(e);
255+
}
256+
let query: GraphqlQueryObj = serde_json::from_str(&String::from_utf8_lossy(&output.stdout))?;
257+
let first_query = match query.differential_commit_query.first() {
258+
Some(first) => first,
259+
None => {
260+
return Err(format_err!(
261+
"No results were found when checking phabricator"
262+
));
263+
}
264+
};
265+
let nodes = &first_query.results.nodes;
266+
let imported = match nodes.first() {
267+
Some(imp_obj) => imp_obj.imported,
268+
None => return Ok(false),
269+
};
270+
Ok(imported)
271+
}
272+
170273
fn is_valid_bookmark_suffix(bookmark_suffix: &str) -> bool {
171274
let spec_chars = "./-_";
172275
bookmark_suffix
@@ -232,6 +335,39 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
232335
.required(true)
233336
.takes_value(true)
234337
.help("Suffix of the bookmark (repo_import_<suffix>)"),
338+
)
339+
.arg(
340+
Arg::with_name(ARG_CALL_SIGN)
341+
.long(ARG_CALL_SIGN)
342+
.takes_value(true)
343+
.help("Call sign to get commit info from Phabricator. e.g. FBS for fbsource"),
344+
)
345+
.arg(
346+
Arg::with_name(ARG_PHAB_CHECK_DISABLED)
347+
.long(ARG_PHAB_CHECK_DISABLED)
348+
.takes_value(false)
349+
.help("Disable waiting for Phabricator to parse commits."),
350+
)
351+
.arg(
352+
Arg::with_name(ARG_X_REPO_CHECK_DISABLED)
353+
.long(ARG_X_REPO_CHECK_DISABLED)
354+
.takes_value(false)
355+
.help("Disable x_repo sync check after moving the bookmark"),
356+
)
357+
.arg(
358+
Arg::with_name(ARG_HG_SYNC_CHECK_DISABLED)
359+
.long(ARG_HG_SYNC_CHECK_DISABLED)
360+
.takes_value(false)
361+
.help("Disable hg sync check after moving the bookmark"),
362+
)
363+
.arg(
364+
Arg::with_name(ARG_SLEEP_TIME)
365+
.long(ARG_SLEEP_TIME)
366+
.takes_value(true)
367+
.default_value("1")
368+
.help(
369+
"Sleep time, if we fail dependent system (phabricator, hg_sync ...) checkers",
370+
),
235371
);
236372

237373
let matches = app.get_matches();
@@ -240,14 +376,30 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
240376
let prefix = matches.value_of(ARG_DEST_PATH).unwrap();
241377
let bookmark_suffix = matches.value_of(ARG_BOOKMARK_SUFFIX).unwrap();
242378
let batch_size = matches.value_of(ARG_BATCH_SIZE).unwrap();
243-
let batch_size = batch_size.parse::<NonZeroUsize>()?;
379+
let batch_size = batch_size.parse::<NonZeroUsize>()?.get();
244380
if !is_valid_bookmark_suffix(&bookmark_suffix) {
245381
return Err(format_err!(
246382
"The bookmark suffix contains invalid character(s).
247383
You can only use alphanumeric and \"./-_\" characters"
248384
));
249385
}
250386

387+
let phab_check_disabled = matches.is_present(ARG_PHAB_CHECK_DISABLED);
388+
let x_repo_check_disabled = matches.is_present(ARG_X_REPO_CHECK_DISABLED);
389+
let hg_sync_check_disabled = matches.is_present(ARG_HG_SYNC_CHECK_DISABLED);
390+
let call_sign = matches.value_of(ARG_CALL_SIGN);
391+
if !phab_check_disabled && call_sign.is_none() {
392+
return Err(format_err!("Call sign was not specified"));
393+
}
394+
let checker_flags = CheckerFlags {
395+
phab_check_disabled,
396+
x_repo_check_disabled,
397+
hg_sync_check_disabled,
398+
call_sign,
399+
};
400+
let sleep_time = matches.value_of(ARG_SLEEP_TIME).unwrap();
401+
let sleep_time = sleep_time.parse::<u64>()?;
402+
251403
args::init_cachelib(fb, &matches, None);
252404

253405
let logger = args::init_logging(fb, &matches);
@@ -263,8 +415,10 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
263415
&ctx,
264416
&repo,
265417
&shifted_bcs,
266-
batch_size.get(),
418+
batch_size,
267419
&bookmark_suffix,
420+
&checker_flags,
421+
sleep_time,
268422
)
269423
.await
270424
},
@@ -278,8 +432,7 @@ fn main(fb: FacebookInit) -> Result<(), Error> {
278432

279433
#[cfg(test)]
280434
mod tests {
281-
use crate::move_bookmark;
282-
use crate::sort_bcs;
435+
use crate::{move_bookmark, sort_bcs, CheckerFlags};
283436

284437
use anyhow::Result;
285438
use blobstore::Loadable;
@@ -294,6 +447,14 @@ mod tests {
294447
let ctx = CoreContext::test_mock(fb);
295448
let blob_repo = blobrepo_factory::new_memblob_empty(None)?;
296449
let batch_size: usize = 2;
450+
let call_sign = Some("FBS");
451+
let checker_flags = CheckerFlags {
452+
phab_check_disabled: true,
453+
x_repo_check_disabled: true,
454+
hg_sync_check_disabled: true,
455+
call_sign,
456+
};
457+
let sleep_time = 1;
297458
let changesets = create_from_dag(
298459
&ctx,
299460
&blob_repo,
@@ -307,7 +468,16 @@ mod tests {
307468
bonsais.push(csid.load(ctx.clone(), &blob_repo.get_blobstore()).await?);
308469
}
309470
bonsais = sort_bcs(&bonsais)?;
310-
move_bookmark(&ctx, &blob_repo, &bonsais, batch_size, "test_repo").await?;
471+
move_bookmark(
472+
&ctx,
473+
&blob_repo,
474+
&bonsais,
475+
batch_size,
476+
"test_repo",
477+
&checker_flags,
478+
sleep_time,
479+
)
480+
.await?;
311481
// Check the bookmark moves created BookmarkLogUpdate entries
312482
let entries = blob_repo
313483
.attribute_expected::<dyn BookmarkUpdateLog>()

eden/mononoke/tests/integration/test-repo_import.t

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
# Import it into Mononoke
3737
$ cd "$TESTTMP"
38-
$ repo_import "$GIT_REPO" --dest-path "new_dir/new_repo" --batch-size 3 --bookmark-suffix "new_repo"
38+
$ repo_import "$GIT_REPO" --dest-path "new_dir/new_repo" --batch-size 3 --bookmark-suffix "new_repo" --disable-phabricator-check
3939
* using repo "repo" repoid RepositoryId(0) (glob)
4040
* Created ce435b03d4ef526648f8654c61e26ae5cc1069cc => ChangesetId(Blake2(f7cbf75d9c08ff96896ed2cebd0327aa514e58b1dd9901d50129b9e08f4aa062)) (glob)
4141
* Created 2c01e4a5658421e2bfcd08e31d9b69399319bcd3 => ChangesetId(Blake2(f7708ed066b1c23591f862148e0386ec704a450e572154cc52f87ca0e394a0fb)) (glob)

0 commit comments

Comments
 (0)