Skip to content

Commit

Permalink
Enable the oximeter producer garbage collection RPW (#5764)
Browse files Browse the repository at this point in the history
Closes #5284.
  • Loading branch information
jgallagher authored May 15, 2024
1 parent 14dbab7 commit 26aba45
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 41 deletions.
2 changes: 1 addition & 1 deletion dev-tools/omdb/tests/successes.out
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ task: "metrics_producer_gc"
currently executing: no
last completed activation: <REDACTED ITERATIONS>, triggered by an explicit signal
started at <REDACTED TIMESTAMP> (<REDACTED DURATION>s ago) and ran for <REDACTED DURATION>ms
last completion reported error: metric producer gc disabled (omicron#5284)
warning: unknown background task: "metrics_producer_gc" (don't know how to interpret details: Object {"expiration": String("<REDACTED TIMESTAMP>"), "pruned": Array []})

task: "phantom_disks"
configured period: every 30s
Expand Down
41 changes: 5 additions & 36 deletions nexus/src/app/background/metrics_producer_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,14 @@ use std::time::Duration;
pub struct MetricProducerGc {
datastore: Arc<DataStore>,
lease_duration: Duration,
disabled: bool,
}

impl MetricProducerGc {
pub fn new(datastore: Arc<DataStore>, lease_duration: Duration) -> Self {
Self {
datastore,
lease_duration,
// TODO We should turn this task on as a part of landing the rest of
// the move to metric producer leases. For now, we leave it disabled
// to avoid pruning producers that don't know to renew leases, but
// make this a boolean so our unit test can enable it.
disabled: true,
}
Self { datastore, lease_duration }
}

async fn activate(&mut self, opctx: &OpContext) -> serde_json::Value {
if self.disabled {
warn!(
opctx.log,
"Metric producer GC: statically disabled pending omicron#5284"
);
return json!({
"error": "metric producer gc disabled (omicron#5284)",
});
}

let Some(expiration) = TimeDelta::from_std(self.lease_duration)
.ok()
.and_then(|delta| Utc::now().checked_sub_signed(delta))
Expand Down Expand Up @@ -219,25 +200,13 @@ mod tests {
.await
.expect("failed to insert producer");

// Activate the task. It should immediately return because our GC is
// currently statically disabled (remove this check once that is no
// longer true!).
// Create the task and activate it. Technically this is racy in that it
// could prune the producer we just added, but if it's been an hour
// since then, we have bigger problems. This should _not_ prune the
// producer, since it's been active within the last hour.
let mut gc =
MetricProducerGc::new(datastore.clone(), Duration::from_secs(3600));
let value = gc.activate(&opctx).await;
assert_eq!(
value,
json!({
"error": "metric producer gc disabled (omicron#5284)",
})
);

// Enable the task and activate it. Technically this is racy, but if
// it's been an hour since we inserted the producer in the previous
// statement, we have bigger problems. This should _not_ prune the
// producer, since it's been active within the last hour.
gc.disabled = false;
let value = gc.activate(&opctx).await;
let value = value.as_object().expect("non-object");
assert!(!value.contains_key("failures"));
assert!(value.contains_key("expiration"));
Expand Down
25 changes: 21 additions & 4 deletions test-utils/src/dev/test_cmds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,27 @@ pub fn redact_variable(input: &str) -> String {
.replace_all(&s, "<REDACTED_TIMESTAMP>")
.to_string();

let s = regex::Regex::new(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z")
.unwrap()
.replace_all(&s, "<REDACTED TIMESTAMP>")
.to_string();
let s = {
let mut new_s = String::with_capacity(s.len());
let mut last_match = 0;
for m in regex::Regex::new(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z")
.unwrap()
.find_iter(&s)
{
new_s.push_str(&s[last_match..m.start()]);
new_s.push_str("<REDACTED");
// We know from our regex that `m.len()` is at least 2 greater than
// the length of "<REDACTEDTIMESTAMP>", so this subtraction can't
// underflow. Insert spaces to match widths.
for _ in 0..(m.len() - "<REDACTEDTIMESTAMP>".len()) {
new_s.push(' ');
}
new_s.push_str("TIMESTAMP>");
last_match = m.end();
}
new_s.push_str(&s[last_match..]);
new_s
};

// Replace formatted durations. These are pretty specific to the background
// task output.
Expand Down

0 comments on commit 26aba45

Please sign in to comment.