ci: prevent sbt bootstrap Maven rate limits - #2581
Open
ranadeepsingh wants to merge 8 commits into
Open
Conversation
## Summary Require lower-is-better validation metrics to improve by more than improvementTolerance before resetting the early-stopping counter. Clarify the parameter documentation and add focused regression coverage for both metric directions and zero tolerance. ## Prompting Intent Investigate GitHub issue microsoft#2565 from a new branch based on master, determine whether the report is valid, and implement a complete fix suitable for an upstream SynapseML pull request. ## Linked Sources - GitHub issue: microsoft#2565 ## Rationale The existing higher-is-better comparison already treats improvementTolerance as a minimum delta, while lower-is-better metrics accepted small regressions. A package-internal comparison helper makes the intended symmetric behavior directly testable without adding a slow native LightGBM fixture or changing public APIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary Expand improvement-tolerance coverage across representative LightGBM metrics and tolerance values. Preserve disabled early stopping when earlyStoppingRound is zero, validate both early-stopping parameters, and document their accepted ranges. ## Prompting Intent The engineer requested broader parameter testing to ensure the issue microsoft#2565 fix does not introduce downstream regressions. Cover related defaults, boundaries, metric families, invalid values, and early-stopping-round interactions before updating the pull request. ## Linked Sources - GitHub issue: microsoft#2565 - Pull request: microsoft#2578 - LightGBM 3.3.5 parameters: https://lightgbm.readthedocs.io/en/v3.3.5/Parameters.html#early-stopping-round ## Rationale Correct tolerance semantics classify more rounds as non-improving, so the wrapper must explicitly preserve LightGBM's zero-means-disabled behavior. Shared Spark parameter validators reject values that LightGBM does not support, while deterministic matrix tests cover the decision logic without depending on platform-specific native binaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SynapseML's Azure Pipelines fans out ~30 hosted-agent matrix jobs that each cold-bootstrap the sbt launcher (org.scala-sbt:sbt:1.10.11, pinned in project/build.properties) and resolve Ivy dependencies from public Maven Central. When many fresh agents -- and several overlapping PR builds -- do this simultaneously, Maven Central returns HTTP 429 (rate limit) and "Setup repo" fails before any test runs (e.g. ADO build 229124511, UnitTests flaky). The pre-existing jittered retries only widened the window against a sustained throttle; they did not remove the thundering herd. Durable fix (cache-first, stagger as supplement): * templates/sbt_cache.yml (primary): Azure Cache@2 for the sbt launcher boot dir (~/.sbt/boot -- the artifact that 429s) and the Ivy cache (~/.ivy2/cache). In steady state, jobs restore these from Azure's cache service and never touch Maven Central. Keys derive from the bootstrap inputs (project/build.properties, project/plugins.sbt, build.sbt) so they invalidate exactly when those change; restoreKeys give a safe partial fallback and continueOnError keeps a cache miss/corruption non-fatal. * BuildAndCacheSbt prewarm job: warms those caches once per run, mirroring the existing BuildAndCacheCondaEnv job. * tools/ci/sbt_retry.sh: single tested helper replacing the duplicated inline retry blocks. Smooths only the cold-cache path with a bounded random start stagger (desynchronises concurrent cold bootstraps) plus bounded jittered exponential-backoff retries. Fails visibly on exhaustion -- no success fallback masking. Wired the shared cache template into every sbt-running job (Style, Publish, Databricks/Fabric E2E, BuildDocker, PythonTests, RTests, WebsiteSamplesTests, UnitTests, ReleaseBranchCompat) by reviving the dormant ivy_cache placeholders, and routed all `sbt setup` bootstraps through the helper. Tests (python -m pytest tools/ci/tests/): deterministically exercise the retry/backoff/stagger + visible-failure behaviour with a fake sbt, and assert pipeline.yaml parses, the cache keys invalidate on bootstrap inputs, and every sbt job is wired to the cache template + prewarm job. No LightGBM, Isolation Forest, GPU, or application changes. TLS verification, job coverage, and all tests are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey @ranadeepsingh 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens SynapseML’s Azure Pipelines sbt bootstrap path to reduce Maven Central HTTP 429 failures by restoring sbt/ivy artifacts from Azure Cache and centralizing the cold-cache retry/stagger logic. The current diff also includes stacked LightGBM early-stopping tolerance fixes from #2578 (per PR description).
Changes:
- Introduces a shared
Cache@2template for~/.sbt/bootand~/.ivy2/cache, and wires it into sbt-running Azure Pipeline jobs. - Adds
tools/ci/sbt_retry.shto provide bounded start stagger + bounded jittered exponential-backoff retries, and replaces duplicated inline retry snippets in the pipeline. - Adds deterministic CI tests validating both
sbt_retry.shbehavior and thatpipeline.yamlis wired to the cache template + prewarm job.
Show a summary per file
| File | Description |
|---|---|
| tools/ci/tests/test_sbt_retry.py | Deterministic pytest coverage for sbt_retry.sh (stagger/backoff/retry/arg forwarding). |
| tools/ci/tests/test_pipeline_yaml.py | YAML-based wiring assertions for pipeline.yaml + templates (presence, parsing, cache usage, retry helper usage). |
| tools/ci/sbt_retry.sh | New retry/stagger wrapper for sbt bootstrap in CI. |
| tools/ci/README.md | Documents the CI bootstrap caching + retry approach and how to run tests. |
| templates/sbt_cache.yml | New reusable Cache@2 template for sbt launcher boot + Ivy cache. |
| pipeline.yaml | Adds sbt cache template usage, adds BuildAndCacheSbt prewarm job, and switches sbt setup calls to the retry helper. |
| lightgbm/src/test/scala/com/microsoft/azure/synapse/ml/lightgbm/split1/TrainUtilsSuite.scala | Tests for LightGBM improvement tolerance symmetry and early stopping parameter validation. |
| lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/TrainUtils.scala | Refactors early-stopping logic into helpers and fixes improvement comparison semantics. |
| lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/params/LightGBMParams.scala | Tightens param docs and adds non-negative validators for early stopping round and improvement tolerance. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
## Summary Make the sbt bootstrap prewarm a mandatory gate before Azure Pipeline matrix jobs start. Add Coursier caching, require exact hits on the boot, Ivy, and Coursier caches before disabling the cold-cache stagger, wire the conditional release job, and strengthen pipeline tests around the dependency graph and cache lifecycle. ## Prompting Intent The engineer asked to fix Maven Central HTTP 429 setup failures in a new stacked PR. The solution must prevent fresh hosted agents from cold-bootstrapping sbt concurrently, allow at least the existing job fan-out after bootstrap is safe, retain bounded retry behavior for cache-service failures, and keep bootstrap failures visible rather than masking them. ## Linked Sources - Failing Azure job: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229124511&view=logs&jobId=e97036a1-bcdb-5cd5-905e-b0cf2c8f33cf - Parent PR investigation: microsoft#2578 (comment) - Stacked PR: microsoft#2581 - Prewarm concurrency review: microsoft#2581 (comment) ## Rationale A best-effort prewarm running beside the matrix does not protect the first run for a new dependency key, so every sbt-running job now waits for one successful warm job. Cache-service errors remain non-fatal and fall back to staggered retries, but a failed warm blocks fan-out to avoid recreating the thundering herd. Coursier is cached alongside sbt boot and Ivy because modern resolution uses all three stores, and the stagger is suppressed only when every cache is an exact hit so dependency-only changes remain desynchronized. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2581 +/- ##
==========================================
- Coverage 84.79% 84.29% -0.51%
==========================================
Files 334 334
Lines 17806 17806
Branches 1623 1623
==========================================
- Hits 15099 15009 -90
- Misses 2707 2797 +90 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
## Summary Add conservative pull-request impact detection for the six-leg Databricks E2E matrix. Clearly non-impacting documentation, website, GitHub metadata, CI helper, and isolated test-source changes skip Databricks, while all uncertain or runtime-affecting changes continue to run it. ## Prompting Intent The engineer asked to extend PR microsoft#2581 so expensive Databricks Azure Pipeline jobs are skipped when the pull request cannot affect notebook execution. The gate must preserve scheduled and branch coverage, avoid brittle CPU-shard mapping, and default to running whenever impact detection is incomplete or uncertain. ## Linked Sources - Stacked CI PR: microsoft#2581 - Full green baseline build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229164855 - Azure multi-job output variables: https://learn.microsoft.com/en-us/azure/devops/pipelines/process/conditions?view=azure-devops#job-output-variables-used-in-other-job-conditions ## Rationale Use one fail-open decision for the complete Databricks matrix because the five CPU partitions mix notebooks across modules and are not stable ownership boundaries. The detector skips only a narrow allowlist of clearly inert paths; runtime code, notebooks, build and pipeline files, Databricks test utilities, shared TestBase infrastructure, unknown paths, empty diffs, and fetch or classifier failures all keep E2E enabled. Non-PR builds always run to preserve scheduled and release coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
Author
|
Validated the Databricks impact gate in Azure build 229178168:
Local deterministic coverage for the skip branch also passes: docs/website/GitHub metadata/CI-helper/non-Databricks test-only changes return |
Collaborator
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
5 tasks
This was referenced Aug 1, 2026
ranadeepsingh
added a commit
that referenced
this pull request
Aug 1, 2026
## Summary Classify changed paths against the actual Databricks CPU and GPU runtime surfaces, emit separate fail-open decisions, and gate each matrix leg independently. ## Prompting Intent The engineer asked to determine exactly when Databricks tests should run, lock down the path rules, and deliver the work as a stacked pull request above PR #2581. ## Linked Sources - Base CI hardening PR: #2581 - GitHub stacked PR documentation: https://docs.github.com/en/pull-requests/how-tos/create-pull-requests/creating-stacked-pull-requests - ADO timing audit: build 229176406 ## Rationale CPU and GPU decisions are separated because most module changes cannot affect the expensive GPU notebooks. Unknown paths and shared build or test infrastructure remain fail-open, while explicit test-only and unrelated tooling paths skip safely. This preserves coverage while avoiding unrelated GPU capacity waits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ranadeepsingh
added a commit
that referenced
this pull request
Aug 1, 2026
## Summary Run release compatibility checks for both GitHub target-branch formats and replace redundant compile, setup, credential, and per-package SBT tasks with one cached, project-scoped validation process. ## Prompting Intent The engineer asked to fix the silently skipped ReleaseBranchCompat job and simplify it before enabling it so the check is both reliable and efficient. ## Linked Sources - Base CI hardening PR: #2581 - Evidence build with skipped phase: ADO build 229176406 - Parent stack layer: ci/databricks-impact-gating ## Rationale The target condition accepts both values observed across Azure Repos and GitHub PR providers. A single SBT process retains full test compilation and the intended core, VW, and OpenCV compatibility suites while removing repeated build loading, root-wide IntelliJ setup, unnecessary Key Vault access, and Azure CLI authentication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ranadeepsingh
added a commit
that referenced
this pull request
Aug 1, 2026
Make the weekly ACR cleanup schedule-only, switch it to the dedicated cleanup service connection, replace storage connection-string authentication with Azure CLI Entra authentication, and add fail-safe cleanup tests. The engineer asked to repair the weekly cleanup failures caused by disabled key-based storage authentication, use the declared least-privileged identity, and prevent accidental CI or PR execution. - Failed scheduled build: ADO build 228250033 - Base CI hardening PR: #2581 - Azure CLI pipeline-run reference: https://learn.microsoft.com/en-us/cli/azure/acr/pipeline-run - Parent stack layer: ci/release-branch-compat Using az storage blob exists with auth-mode login keeps all operations inside the AzureCLI task identity and removes runtime SDK installation, Key Vault access, and storage keys. Images are deleted only after the archive is confirmed, and subprocess argument lists avoid shell interpolation of registry-controlled names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ranadeepsingh
added a commit
that referenced
this pull request
Aug 1, 2026
## Summary Run non-Azure setup and coverage commands as Bash steps, install pinned Black without restoring the 8.6 GB Conda environment, and remove the ineffective standalone Conda cache consumer. ## Prompting Intent The engineer asked for additional improvements that should ship with the requested CI fixes to make builds faster and more reliable without broad behavioral changes. ## Linked Sources - CI efficiency audit from ADO build 229176406 - Base CI hardening PR: #2581 - Parent stack layer: ci/fix-acr-cleanup-auth ## Rationale AzureCLI tasks create an isolated login for every invocation, so setup and coverage steps that never call az gain no authentication benefit. The Style job only needs pinned Black, not the full cached environment. The standalone Conda job was not a dependency and therefore could not prewarm consumers or prevent cold-cache fan-out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.
CI bootstrap hardening and selective Databricks validation
Parent PR #2578 is merged, and this PR now has an isolated CI-only diff against
master.Commits:
0f57ab2a03— cache-first sbt bootstrap and bounded retry helper3bcccca803— mandatory prewarm gate, Coursier cache, exact-hit handling, and stronger pipeline tests17e6bd1d68— conservative PR impact gating for the six Databricks E2E matrix legsRoot cause
SynapseML's Azure Pipeline fans out roughly 30 hosted-agent jobs. Fresh agents concurrently downloaded the sbt 1.10.11 launcher and dependencies from public Maven Central. In build 229124511,
UnitTests flakyexhausted six setup attempts on HTTP 429 before any test ran. Retries alone did not remove the thundering herd.Cache-first design
BuildAndCacheSbtgate: one job restores or populates the exact cache keys before fan-out. Every sbt-running job waits for it.Cache@2stores:~/.sbt/boot,~/.ivy2/cache, and~/.cache/coursier, with versioned keys derived from sbt and dependency definitions.tools/ci/sbt_retry.shprovides per-attempt timeout, random initial staggering, jittered exponential backoff, argument forwarding, and visible final failure.Cache-service failures remain non-fatal, but a failed prewarm blocks fan-out to avoid another stampede.
Databricks E2E impact gating
For pull requests,
BuildAndCacheSbtdiffs the Azure PR merge ref against the current target branch and emits a multi-job output consumed byDatabricksE2E.All six Databricks matrix legs skip only when every changed path is clearly non-impacting:
tools/ci/helpersThe detector is intentionally fail-open. Runtime code,
.ipynbnotebooks, build definitions, pipeline/templates, Databricks test utilities, sharedTestBase, unknown paths, empty diffs, or detection/fetch failures all run Databricks. Scheduled, master, tag, and manual builds always run. The five CPU partitions mix notebooks across modules, so this avoids unsafe per-shard ownership assumptions.Validation
UnitTests flaky,UnitTests isolationforest, OpenCV, all five Databricks CPU legs, and Databricks GPUsbt updatefirst try in 27 secondspython -m pytest tools/ci/tests -q→ 22 passedblack --check --extend-exclude 'docs/' .→ 183 files unchangedsbt scalastyle test:scalastylewith JDK 11 → passedbash -n tools/ci/sbt_retry.sh, andgit diff --check→ passedScope
CI/bootstrap infrastructure only. No LightGBM, Isolation Forest, GPU workload, notebook, or application behavior changes.