ci(bazel): restore change-aware matrix scheduling#424
Merged
Conversation
Drop the blanket run_all=true override so the detect job schedules only the subtrees a change actually touches. Each subtree is an isolated Bazel module, so a change under one service builds+tests only that service's //..., never another module. A docs-only or helm-only change selects zero rows (the verification gate passes on any=false). Reverse-dependency edges are unchanged: a Java framework change still fans out to every Java service, shared Java/root infra still schedules every Java row, and ROOT_GLOBS still select the root row. workflow_dispatch, a change to this workflow, and unreadable/rewritten/new-branch diffs still fall back to the full matrix. This ends full-matrix rebuilds on unrelated PRs (e.g. a docs-only change rebuilding every service). Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
📝 WalkthroughWalkthroughThe Bazel workflow now uses existing change-aware detection to select affected subtree modules and Java reverse-dependency rows, while retaining full-matrix fallbacks for dispatches, workflow changes, and unsafe or unknown diffs. ChangesBazel workflow scheduling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/bazel.yml:
- Around line 174-184: Replace every pipelined printf-to-grep path match in the
change-aware scheduling logic with a non-pipelined check against the
changed-path data, such as a here-string passed to grep with fixed-string
matching. Update the workflow, framework, and subtree tests while preserving
their existing match semantics and scheduling behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d5529ef2-db6e-41da-88ec-4aa3ec799354
📒 Files selected for processing (1)
.github/workflows/bazel.yml
sbaum1994
approved these changes
Jul 24, 2026
kristinapathak
approved these changes
Jul 24, 2026
kristinapathak
enabled auto-merge
July 24, 2026 16:53
vrv3814
pushed a commit
to mesutoezdil/nvcf
that referenced
this pull request
Jul 25, 2026
* ci(bazel): harden detect matching + cap matrix concurrency Two fixes for the change-aware detect job restored in NVIDIA#424: 1. Replace `printf ... | grep -q` with a here-string (`grep -q PATTERN <<<"$changed"`). Under `set -o pipefail`, grep -q closes the pipe on its first match and printf can take SIGPIPE (141); pipefail then makes the pipeline non-zero, so a real match reads as no-match and the subtree is silently dropped from the matrix. The here-string removes the pipe. (CodeRabbit finding on NVIDIA#424.) 2. Add max-parallel: 8 to the bazel matrix. A full matrix otherwise starts ~20 jobs at once and every runner pulls actions/checkout from codeload.github.com in the same instant, tripping GitHub's action download rate limit (HTTP 429 in Set up job). Batching keeps the burst under the limit; change-aware scheduling keeps most PRs below the cap. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com> * ci(bazel): cap docker lane concurrency + rename the merge-gate job - Add max-parallel to the bazel-docker matrix so the docker-host rows do not add to the simultaneous actions/checkout burst (companion to the cap on the container bazel job). - Rename the aggregate gate job from "bazel verification" to "bazel required checks" so it is recognizable as the single status check to require for merge. Under change-aware scheduling the individual bazel (<service>) rows are dynamic and must not be required; this always-run rollup is the stable gate. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Since #343 merged, every PR rebuilds and tests every service, including docs-only changes like #423. #343 (commit f286e51) added a blanket
run_all=trueoverride on top of the change-aware scheduler that had been working since commit 126c56b (feat(ci): change-aware GHA bazel matrix). The override was a safety net while debugging cloud-tasks test output during that import, and is no longer needed.Each subtree is an isolated Bazel module, so a change under one service builds and tests only that service's
//...and cannot break another module. Forcing the full matrix on unrelated changes just burns queue time (and pays a cold rebuild on thebazel-dockerlanes, which only warm on main pushes).What changed
Remove the unconditional
run_all=trueoverride in thedetectjob, restoring change-aware scheduling. Everything else is intentionally unchanged:ROOT_GLOBSstill select the root row.workflow_dispatch, a change to this workflow itself, and unreadable/rewritten/new-branch diffs.bazel-verificationgate passes onany=false.Customer Release Notes
Not customer visible (CI-only).
Testing
Traced the
detectselection logic against representative changesets:any=false.YAML validated with a parser; the unconditional
run_all=trueassignment is removed while the conditional fallbacks remain.References
Override introduced in #343 (f286e51). Change-aware scheduling originally landed in 126c56b.
Summary by CodeRabbit