Skip to content

Commit 615196c

Browse files
captbaritonemeta-codesync[bot]
authored andcommitted
Allow fractional percentages in rollout feature flags
Differential Revision: D126486193 D126486193 Reviewed By: tyao1 fbshipit-source-id: 6efe56f0b4db91516235dc0ffb29eb8aee6c451d
1 parent 52c2e68 commit 615196c

6 files changed

Lines changed: 132 additions & 23 deletions

File tree

‎compiler/Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎compiler/crates/common/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ rayon = "1.11.0"
2323
schemars = { version = "1.2.2", features = ["indexmap2"] }
2424
serde = { version = "1.0.229", features = ["derive", "rc"] }
2525
serde_json = { version = "1.0.151", features = ["alloc", "float_roundtrip", "raw_value", "unbounded_depth"] }
26+
thiserror = "2.0.21"
2627
typetag = "0.2.23"
2728

2829
[dev-dependencies]

‎compiler/crates/common/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ pub use perf_logger::NoopPerfLoggerEvent;
6262
pub use perf_logger::PerfLogEvent;
6363
pub use perf_logger::PerfLogger;
6464
pub use pointer_address::PointerAddress;
65+
pub use rollout::InvalidRolloutPercentage;
6566
pub use rollout::Rollout;
6667
pub use rollout::RolloutRange;
6768
pub use span::Span;

‎compiler/crates/common/src/rollout.rs‎

Lines changed: 122 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,13 @@ use md5::Md5;
1010
use schemars::JsonSchema;
1111
use serde::Deserialize;
1212
use serde::Serialize;
13+
use thiserror::Error;
1314

1415
/// A utility to enable gradual rollout of large codegen changes.
1516
/// Can be constructed as the Default which passes or a percentage between 0 and
16-
/// 100.
17+
/// 100. Percentages may be fractional with up to two decimal places (e.g.
18+
/// `12.5`), which allows rolling out a change to very large projects in steps
19+
/// smaller than 1%.
1720
#[derive(
1821
Default,
1922
Debug,
@@ -24,20 +27,40 @@ use serde::Serialize;
2427
PartialEq,
2528
JsonSchema
2629
)]
27-
pub struct Rollout(pub Option<u8>);
30+
#[serde(try_from = "Option<f32>")]
31+
#[schemars(extend("minimum" = 0, "maximum" = 100))]
32+
pub struct Rollout(pub Option<f32>);
33+
34+
/// A rollout percentage outside 0 to 100 (or NaN).
35+
#[derive(Debug, Error)]
36+
#[error("Expected a rollout percentage between 0 and 100, inclusive, but got {0}")]
37+
pub struct InvalidRolloutPercentage(f32);
38+
39+
impl TryFrom<Option<f32>> for Rollout {
40+
type Error = InvalidRolloutPercentage;
41+
42+
fn try_from(percentage: Option<f32>) -> Result<Self, Self::Error> {
43+
match percentage {
44+
Some(p) if !(0.0..=100.0).contains(&p) => Err(InvalidRolloutPercentage(p)),
45+
_ => Ok(Self(percentage)),
46+
}
47+
}
48+
}
2849

2950
impl Rollout {
3051
/// Checks some key deterministically and passes on average the given
3152
/// percentage of the rollout.
3253
/// A typical key to pass in could be the fragment or operation name.
3354
pub fn check(&self, key: impl AsRef<[u8]>) -> bool {
34-
if let Some(percentage) = self.0 {
35-
let hash = Md5::digest(key.as_ref());
36-
let hash: u16 = ((hash[1] as u16) << 8) | (hash[0] as u16);
37-
(hash % 100) < (percentage as u16)
38-
} else {
39-
true
40-
}
55+
let Some(percentage) = self.0 else {
56+
return true;
57+
};
58+
let hash = Md5::digest(key.as_ref());
59+
// The whole-percent bucket comes first, so a whole-number percentage
60+
// selects exactly the keys it did when rollouts were integers.
61+
let bucket = u32::from(u16::from_le_bytes([hash[0], hash[1]]) % 100) * 100
62+
+ u32::from(u16::from_le_bytes([hash[2], hash[3]]) % 100);
63+
bucket < (percentage * 100.0).round() as u32
4164
}
4265
}
4366

@@ -60,3 +83,93 @@ impl RolloutRange {
6083
(percent) <= (self.end as u16) && (percent) >= (self.start as u16)
6184
}
6285
}
86+
87+
#[cfg(test)]
88+
mod tests {
89+
use super::*;
90+
91+
fn keys() -> impl Iterator<Item = String> {
92+
(0..20_000).map(|i| format!("Key{i}"))
93+
}
94+
95+
/// The bucketing used before fractional percentages were supported.
96+
fn legacy_check(percentage: u8, key: &str) -> bool {
97+
let hash = Md5::digest(key.as_bytes());
98+
let hash: u16 = ((hash[1] as u16) << 8) | (hash[0] as u16);
99+
(hash % 100) < (percentage as u16)
100+
}
101+
102+
#[test]
103+
fn whole_percentages_select_the_same_keys_as_before() {
104+
for percentage in [0u8, 1, 37, 50, 99, 100] {
105+
let rollout = Rollout(Some(percentage as f32));
106+
for key in keys() {
107+
assert_eq!(
108+
rollout.check(&key),
109+
legacy_check(percentage, &key),
110+
"{percentage}% should select {key} exactly as the integer rollout did",
111+
);
112+
}
113+
}
114+
}
115+
116+
#[test]
117+
fn fractional_percentage_is_between_its_neighbors() {
118+
let lower = Rollout(Some(37.0));
119+
let fractional = Rollout(Some(37.5));
120+
let upper = Rollout(Some(38.0));
121+
for key in keys() {
122+
// A key in 37% is also in 37.5%, and a key in 37.5% is also in 38%.
123+
assert!(!lower.check(&key) || fractional.check(&key), "{key}");
124+
assert!(!fractional.check(&key) || upper.check(&key), "{key}");
125+
}
126+
}
127+
128+
#[test]
129+
fn fractional_percentage_selects_roughly_that_share() {
130+
let count = keys().filter(|key| Rollout(Some(37.5)).check(key)).count();
131+
// 37.5% of 20,000 keys is 7,500.
132+
assert!((7_200..=7_800).contains(&count), "selected {count}");
133+
}
134+
135+
#[test]
136+
fn half_percent_selects_part_of_the_first_bucket() {
137+
let half = keys().filter(|key| Rollout(Some(0.5)).check(key)).count();
138+
let one = keys().filter(|key| Rollout(Some(1.0)).check(key)).count();
139+
// 0.5% of 20,000 keys is 100; 1% is 200.
140+
assert!(
141+
half > 0 && half < one,
142+
"0.5% selected {half}, 1% selected {one}"
143+
);
144+
}
145+
146+
#[test]
147+
fn config_accepts_percentages_from_0_to_100() {
148+
for json in ["0", "0.5", "100", "null"] {
149+
assert!(
150+
serde_json::from_str::<Rollout>(json).is_ok(),
151+
"{json} should parse"
152+
);
153+
}
154+
}
155+
156+
#[test]
157+
fn config_rejects_percentages_outside_0_to_100() {
158+
// A missed decimal point (`1000` for `10.00`) must not silently enable the feature everywhere.
159+
for json in ["-1", "100.5", "1000"] {
160+
assert!(
161+
serde_json::from_str::<Rollout>(json).is_err(),
162+
"{json} should be rejected"
163+
);
164+
}
165+
assert!(
166+
Rollout::try_from(Some(f32::NAN)).is_err(),
167+
"NaN should be rejected"
168+
);
169+
}
170+
171+
#[test]
172+
fn default_rollout_passes_every_key() {
173+
assert!(keys().all(|key| Rollout(None).check(key)));
174+
}
175+
}

‎compiler/crates/relay-codemod/src/codemod.rs‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -299,16 +299,9 @@ fn valid_percent(s: &str) -> Result<FeatureFlag, String> {
299299
Err("numbers must be between 0 and 100, inclusive, and the first number must be less than or equal to the second".to_string())
300300
}
301301
} else {
302-
// turn s into a u8
303-
let s = s.parse::<u8>().map_err(|_| "not a number".to_string())?;
304-
// check if s is less than 100
305-
if (0..=100).contains(&s) {
306-
Ok(FeatureFlag::Rollout {
307-
rollout: Rollout(Some(s)),
308-
})
309-
} else {
310-
Err("number must be between 0 and 100, inclusive".to_string())
311-
}
302+
let s = s.parse::<f32>().map_err(|_| "not a number".to_string())?;
303+
let rollout = Rollout::try_from(Some(s)).map_err(|err| err.to_string())?;
304+
Ok(FeatureFlag::Rollout { rollout })
312305
}
313306
}
314307

‎compiler/crates/relay-compiler/relay-compiler-config-schema.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,13 +1435,13 @@
14351435
"additionalProperties": false
14361436
},
14371437
"Rollout": {
1438-
"description": "A utility to enable gradual rollout of large codegen changes.\nCan be constructed as the Default which passes or a percentage between 0 and\n100.",
1438+
"description": "A utility to enable gradual rollout of large codegen changes.\nCan be constructed as the Default which passes or a percentage between 0 and\n100. Percentages may be fractional with up to two decimal places (e.g.\n`12.5`), which allows rolling out a change to very large projects in steps\nsmaller than 1%.",
14391439
"type": [
1440-
"integer",
1440+
"number",
14411441
"null"
14421442
],
1443-
"format": "uint8",
1444-
"maximum": 255,
1443+
"format": "float",
1444+
"maximum": 100,
14451445
"minimum": 0
14461446
},
14471447
"RolloutRange": {

0 commit comments

Comments
 (0)