mirror of
https://github.com/ruvnet/RuView
synced 2026-07-23 17:33:20 +00:00
fix(adr-186): make exported .rvf filenames process-unique (fixes CI flake)
Root cause: the trained-model filename was `trained-{type}-{%Y%m%d_%H%M%S}` — a
**second-resolution** timestamp. Two training runs of the same type that finish
in the same wall-clock second compute the SAME path and silently overwrite each
other's artifact. This is a real robustness gap (a second rapid run clobbers the
first model), and it surfaced as a flaky test on the Linux CI runner:
`http_train_start_produces_model_and_streams` asserts (via a before/after
directory diff) that a new `.rvf` appeared, but the concurrent
`training_job_streams_real_progress_and_writes_model` test — same `supervised`
type, same second — wrote to the identical path and then deleted it in its own
cleanup, so the diff saw no new file. Windows scheduling happened to avoid the
same-second overlap; Linux CI hit it (the sibling `TRAIN_ENV_LOCK` holders then
failed via std::sync::Mutex poison-cascade after the first panic).
Reproduced locally by isolating the three model-writing tests (which forces the
overlap): ~10/20 runs flaked before; 0/30 after.
Fix: `next_model_id()` appends microseconds (`%6f`) plus a monotonic per-process
`AtomicU64` counter, so every run gets a distinct path even for same-microsecond
concurrent completions. Pinned by `model_ids_are_unique_per_call` (1000 ids, all
distinct) so the scheme can't regress to non-unique. No test-level band-aid
(no added sleeps / serialization) — the collision is removed at the source.
Verified: cargo test -p wifi-densepose-sensing-server -p wifi-densepose-train
--no-default-features — bin 218 passed / 0 failed, all suites 0 failed; the
isolated-tests stress loop is 0/15 flakes post-fix.
This commit is contained in:
@@ -26,7 +26,7 @@
|
|||||||
|
|
||||||
use std::collections::VecDeque;
|
use std::collections::VecDeque;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::sync::atomic::{AtomicBool, Ordering};
|
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
|
||||||
use std::sync::{Arc, Mutex};
|
use std::sync::{Arc, Mutex};
|
||||||
|
|
||||||
use axum::{
|
use axum::{
|
||||||
@@ -54,6 +54,24 @@ pub const MODELS_DIR: &str = "data/models";
|
|||||||
/// `dataset_id` maps to `{RECORDINGS_DIR}/{dataset_id}.csi.jsonl`.
|
/// `dataset_id` maps to `{RECORDINGS_DIR}/{dataset_id}.csi.jsonl`.
|
||||||
pub const RECORDINGS_DIR: &str = "data/recordings";
|
pub const RECORDINGS_DIR: &str = "data/recordings";
|
||||||
|
|
||||||
|
/// Monotonic per-process counter appended to exported model filenames so two
|
||||||
|
/// runs that complete in the same wall-clock microsecond still get distinct
|
||||||
|
/// paths (prevents silent overwrite; keeps concurrent runs from colliding).
|
||||||
|
static MODEL_ID_SEQ: AtomicU64 = AtomicU64::new(0);
|
||||||
|
|
||||||
|
/// Build a process-unique model id `trained-{type}-{ts_micros}-{seq}`. A
|
||||||
|
/// second-resolution timestamp alone collided for runs finishing in the same
|
||||||
|
/// second (silent overwrite); microseconds + the monotonic counter guarantee
|
||||||
|
/// uniqueness even for same-microsecond concurrent completions.
|
||||||
|
fn next_model_id(training_type: &str) -> String {
|
||||||
|
format!(
|
||||||
|
"trained-{}-{}-{}",
|
||||||
|
training_type,
|
||||||
|
chrono::Utc::now().format("%Y%m%d_%H%M%S_%6f"),
|
||||||
|
MODEL_ID_SEQ.fetch_add(1, Ordering::Relaxed)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
/// Number of COCO keypoints.
|
/// Number of COCO keypoints.
|
||||||
const N_KEYPOINTS: usize = 17;
|
const N_KEYPOINTS: usize = 17;
|
||||||
/// Dimensions per keypoint in the target vector (x, y, z).
|
/// Dimensions per keypoint in the target vector (x, y, z).
|
||||||
@@ -1368,11 +1386,7 @@ async fn run_training_job(
|
|||||||
if let Err(e) = tokio::fs::create_dir_all(MODELS_DIR).await {
|
if let Err(e) = tokio::fs::create_dir_all(MODELS_DIR).await {
|
||||||
error!("Failed to create models directory: {e}");
|
error!("Failed to create models directory: {e}");
|
||||||
} else {
|
} else {
|
||||||
let model_id = format!(
|
let model_id = next_model_id(training_type);
|
||||||
"trained-{}-{}",
|
|
||||||
training_type,
|
|
||||||
chrono::Utc::now().format("%Y%m%d_%H%M%S")
|
|
||||||
);
|
|
||||||
let rvf_path = PathBuf::from(MODELS_DIR).join(format!("{model_id}.rvf"));
|
let rvf_path = PathBuf::from(MODELS_DIR).join(format!("{model_id}.rvf"));
|
||||||
|
|
||||||
let mut builder = RvfBuilder::new();
|
let mut builder = RvfBuilder::new();
|
||||||
@@ -2303,6 +2317,19 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Exported model ids must be unique per call — a second-resolution
|
||||||
|
/// timestamp alone collided for runs finishing in the same wall-clock second
|
||||||
|
/// (silently overwriting each other's `.rvf`, which also flaked the
|
||||||
|
/// concurrent model-writing tests on CI). Guards against regressing the
|
||||||
|
/// filename scheme back to non-unique.
|
||||||
|
#[test]
|
||||||
|
fn model_ids_are_unique_per_call() {
|
||||||
|
let ids: Vec<String> = (0..1000).map(|_| next_model_id("supervised")).collect();
|
||||||
|
let unique: std::collections::HashSet<&String> = ids.iter().collect();
|
||||||
|
assert_eq!(unique.len(), ids.len(), "every model id must be distinct");
|
||||||
|
assert!(ids[0].starts_with("trained-supervised-"));
|
||||||
|
}
|
||||||
|
|
||||||
/// ADR-186 P5: the enablement gate is enabled by default and only disabled
|
/// ADR-186 P5: the enablement gate is enabled by default and only disabled
|
||||||
/// by an explicit truthy opt-out, so a `--no-default-features` / default
|
/// by an explicit truthy opt-out, so a `--no-default-features` / default
|
||||||
/// build always has server training ON (no silent regression to disabled).
|
/// build always has server training ON (no silent regression to disabled).
|
||||||
|
|||||||
Reference in New Issue
Block a user