Skip to content

Commit 5b91436

Browse files
RajivTSmeta-codesync[bot]
authored andcommitted
Tag pushrebase Scuba with land_instance_id for MR QE terminal-bounce readout
Summary: The merge-resolution QE needs the ITT *terminal* conflict-bounce (true-rescue) rate, test vs control, computed retry-inclusively. That terminal outcome has to be grouped per land across the Landcastle/Sandcastle client rebase-retry loop, but no existing column on `mononoke_land_service` is stable across that loop: `session_uuid`/`client_correlator` are per push-invocation, `sandcastle_nonce` is per job-type, and `changeset_id` changes on rebase. This adds a stable per-land grouping key -- the Sandcastle land instance id -- to the pushrebase Scuba samples, mirroring how `mr_qe_arm` is stamped (D108612592). The readout then groups `mononoke_land_service` by `land_instance_id` to roll a land's attempts up to a single terminal outcome and slice by arm -- entirely within source_control-owned tables (no www logger / cross-team change). - Landcastle (`LandcastleMrQeBuckets`) emits a `LAND_INSTANCE_ID` pushvar (the Sandcastle instance id) on every (re)push of an enrolled land; the same Sandcastle instance drives the whole `genPush` retry loop, so all attempts of a land carry the same value. - Mononoke reads it into `PushrebaseFlags.land_instance_id` (UTF-8 decode at the bookmarks_movement boundary so `metaconfig_types` stays bytes-free) and stamps `land_instance_id` in the `do_pushrebase_bonsai` `with_mutated_scuba` rebind, so every downstream sample inherits it. Observability only; absent/`None` for out-of-experiment traffic; no behavior change. Reviewed By: YousefSalama, lmvasquezg Differential Revision: D110041743 fbshipit-source-id: f02137439376731a582cc1275c1110ee0f5374d4
1 parent fae36fa commit 5b91436

5 files changed

Lines changed: 19 additions & 0 deletions

File tree

eden/mononoke/features/pushrebase/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@ pub async fn do_pushrebase_bonsai(
333333
// means every downstream `ctx.scuba()` clone inherits the field.
334334
let ctx = ctx.with_mutated_scuba(|mut scuba| {
335335
scuba.add("mr_qe_arm", config.merge_resolution_override.qe_arm_str());
336+
// Per-land key to roll a land's attempts up to a terminal outcome.
337+
if let Some(land_instance_id) = config.land_instance_id.as_deref() {
338+
scuba.add("land_instance_id", land_instance_id);
339+
}
336340
scuba
337341
});
338342
let ctx = &ctx;

eden/mononoke/metaconfig/parser/src/config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,6 +1492,7 @@ mod test {
14921492
merge_resolution_excluded_path_prefixes: Default::default(),
14931493
pessimistic_locking_bookmarks: Vec::new(),
14941494
merge_resolution_override: MergeResolutionOverride::UseJk,
1495+
land_instance_id: None,
14951496
},
14961497
block_merges: false,
14971498
emit_obsmarkers: false,

eden/mononoke/metaconfig/parser/src/convert/repo.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ impl Convert for RawPushrebaseParams {
398398
.map(BookmarkKey::new)
399399
.collect::<Result<Vec<_>>>()?,
400400
merge_resolution_override: MergeResolutionOverride::UseJk, // request-scoped, not loaded from config
401+
land_instance_id: None, // request-scoped, not loaded from config
401402
},
402403
block_merges: self.block_merges.unwrap_or(default.block_merges),
403404
emit_obsmarkers: self.emit_obsmarkers.unwrap_or(default.emit_obsmarkers),

eden/mononoke/metaconfig/types/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,8 +1028,13 @@ pub struct PushrebaseFlags {
10281028
/// never loaded from configerator — set on a cloned `PushrebaseFlags`
10291029
/// before the pushrebase call (mirrors `rewritedates`).
10301030
pub merge_resolution_override: MergeResolutionOverride,
1031+
/// Per-request Sandcastle land instance id (`LAND_INSTANCE_ID` pushvar); stamped on Scuba to group a land's attempts. Observability only.
1032+
pub land_instance_id: Option<String>,
10311033
}
10321034

1035+
/// Pushvar key for the Sandcastle land instance id (see `PushrebaseFlags::land_instance_id`).
1036+
pub const LAND_INSTANCE_ID_PUSHVAR_KEY: &str = "LAND_INSTANCE_ID";
1037+
10331038
/// Per-request override for the `pushrebase_enable_merge_resolution` JustKnob.
10341039
///
10351040
/// `UseJk` (the default) consults the JK as before. `ForceOn`/`ForceOff`
@@ -1164,6 +1169,7 @@ impl Default for PushrebaseFlags {
11641169
merge_resolution_excluded_path_prefixes: PrefixTrie::new(),
11651170
pessimistic_locking_bookmarks: Vec::new(),
11661171
merge_resolution_override: MergeResolutionOverride::UseJk,
1172+
land_instance_id: None,
11671173
}
11681174
}
11691175
}

eden/mononoke/repo_attributes/bookmarks/bookmarks_movement/src/pushrebase_onto.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use context::CoreContext;
1616
use futures_stats::TimedFutureExt;
1717
use hooks::CrossRepoPushSource;
1818
use hooks::HookManager;
19+
use metaconfig_types::LAND_INSTANCE_ID_PUSHVAR_KEY;
1920
use metaconfig_types::MergeResolutionOverride;
2021
use mononoke_types::BonsaiChangeset;
2122
use pushrebase_hook::PushrebaseHook;
@@ -130,6 +131,12 @@ impl<'op> PushrebaseOntoBookmarkOp<'op> {
130131
.and_then(|p| p.get(MergeResolutionOverride::PUSHVAR_KEY))
131132
.map(|b| b.as_ref()),
132133
);
134+
// Per-land key (same on every retry push) for the terminal-bounce readout.
135+
flags.land_instance_id = self
136+
.pushvars
137+
.and_then(|p| p.get(LAND_INSTANCE_ID_PUSHVAR_KEY))
138+
.and_then(|b| std::str::from_utf8(b.as_ref()).ok())
139+
.map(str::to_owned);
133140

134141
ctx.scuba()
135142
.clone()

0 commit comments

Comments
 (0)