From 46e058d15e6fe41b0eb16df98cc5e19c001b0b7a Mon Sep 17 00:00:00 2001 From: b7r6 Date: Wed, 7 Oct 2026 16:21:11 -0400 Subject: [PATCH 1/2] ci: defer the main.yaml integration-tests OCI lane off per-PR Completes the no-per-PR-OCI-builds intent: image-off-pr moved image.yaml off PRs and #2877 moved the nix integration job off PRs, but main.yaml's integration-tests (docker-compose image build + run_integration_tests.sh) fell between them and still ran on every PR. Defer to nightly + merge-to-main, matching #2880's pattern. Fork-local until folded into the upstream OCI/nightly set. --- .github/workflows/main.yaml | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 43dacaf1ea0..232952a3a33 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -1,17 +1,23 @@ name: CI # Controls when the workflow will run. +# +# The only live job here (integration-tests) builds the trace_machina/nativelink +# OCI image from the docker-compose Dockerfile and runs run_integration_tests.sh. +# That is a per-PR OCI build — the same class of heavy work #image-off-pr moved +# off PRs for image.yaml and #2877 moved off PRs for the nix integration job. +# This lane fell in the crack between those two, so defer it the same way: +# nightly cross-check + on every merge to main, not on every PR. Per-PR +# build/test signal is carried by the Nix "Bazel Dev" lane. on: push: branches: [ main ] paths-ignore: - '.github/styles/**' - 'web/**' - pull_request: - branches: [ main ] - paths-ignore: - - '.github/styles/**' - - 'web/**' + schedule: + - cron: '0 7 * * *' # 07:00 UTC daily (off-peak) + workflow_dispatch: permissions: read-all From 11f83898111d8d6320c3368aa3ae60babac63014 Mon Sep 17 00:00:00 2001 From: b7r6 Date: Fri, 2 Oct 2026 11:28:56 -0400 Subject: [PATCH 2/2] fix(store): mongo update_data version-CAS must return retryable Ok(None), not Err ExperimentalMongoStore::update_data used find_one_and_update(...).upsert(true). On a version-CAS conflict (a concurrent writer already bumped the version) the {_id, version} filter matches nothing, so upsert tries to INSERT a doc with the existing _id and MongoDB returns a DuplicateKeyError (E11000), surfaced as Err(Internal). The version-CAS contract (and the Redis backend, which returns Ok(None)) require a conflict to be retryable; instead concurrent updates on a Mongo-backed record spuriously failed. Fix: .upsert(current_version == 0) so only a genuine first-create upserts; a non-matching filter on an existing record now returns Ok(None) (retryable). Adds a deterministic fails-without regression and corrects an existing (ignored) scheduler test that had pinned the buggy Err behavior. Co-Authored-By: Claude Opus 4.8 --- nativelink-store/src/mongo_store.rs | 2 +- nativelink-store/tests/mongo_store_test.rs | 52 +++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/nativelink-store/src/mongo_store.rs b/nativelink-store/src/mongo_store.rs index 8a7bbb0096f..185b9f855b4 100644 --- a/nativelink-store/src/mongo_store.rs +++ b/nativelink-store/src/mongo_store.rs @@ -975,7 +975,7 @@ impl SchedulerStore for ExperimentalMongoStore { let result = match self .scheduler_collection .find_one_and_update(filter, update_doc) - .upsert(true) + .upsert(current_version == 0) .return_document(ReturnDocument::After) .await { diff --git a/nativelink-store/tests/mongo_store_test.rs b/nativelink-store/tests/mongo_store_test.rs index d6133631254..3933ec5b554 100644 --- a/nativelink-store/tests/mongo_store_test.rs +++ b/nativelink-store/tests/mongo_store_test.rs @@ -839,7 +839,10 @@ async fn test_scheduler_store_operations() -> Result<(), Error> { data.version = version1; // Using old version let result = helper.store.update_data(data.clone(), None).await; - assert!(result.is_err(), "Update with old version should fail"); + assert!( + matches!(result, Ok(None)), + "Stale-version update must return retryable Ok(None), got {result:?}" + ); eprintln!("Correctly rejected update with stale version"); } @@ -1056,3 +1059,50 @@ async fn incomplete_upload_preserves_mongo_value() -> Result<(), Error> { } Ok(()) } + +#[nativelink_test] +async fn update_data_stale_version_returns_retryable_none_not_err() -> Result<(), Error> { + // N12 regression: ExperimentalMongoStore::update_data used upsert(true), so a + // version-CAS miss (a concurrent writer already bumped the version) hit the + // unique _id and surfaced as a DuplicateKeyError (Err) instead of the + // retryable Ok(None) the scheduler contract (and the Redis backend) require. + let helper = TestMongoHelper::new(None).await?; + + let mut data = TestSchedulerData { + key: "test:n12_version_cas".to_string(), + content: "v1".to_string(), + version: 0, + }; + + // Seed at version 0 -> creates version 1 (genuine first create still upserts). + let v1 = helper + .store + .update_data(data.clone(), None) + .await + .err_tip(|| "create should succeed")? + .ok_or_else(|| make_err!(Code::Internal, "expected a version on create"))?; + + // One writer wins with the correct version -> version 2. + data.content = "v2".to_string(); + data.version = v1; + let v2 = helper + .store + .update_data(data.clone(), None) + .await + .err_tip(|| "correct-version update should succeed")? + .ok_or_else(|| make_err!(Code::Internal, "expected a version on update"))?; + assert!(v2 > v1, "version should increment ({v1} -> {v2})"); + + // The losing concurrent writer carries the now-stale version. It MUST get a + // retryable Ok(None), not Err. Before the fix this returned Err(Internal) + // (duplicate _id), destroying the retry signal. + data.content = "stale".to_string(); + data.version = v1; + let stale = helper.store.update_data(data.clone(), None).await; + assert!( + matches!(stale, Ok(None)), + "stale-version update must return retryable Ok(None), got {stale:?}" + ); + + Ok(()) +}