Skip to content

ci: validate stacked CI efficiency changes - #2587

Closed
ranadeepsingh wants to merge 12 commits into
masterfrom
ci/stack-validation-2586
Closed

ci: validate stacked CI efficiency changes#2587
ranadeepsingh wants to merge 12 commits into
masterfrom
ci/stack-validation-2586

Conversation

@ranadeepsingh

Copy link
Copy Markdown
Collaborator

Purpose

Temporary validation PR for native stack #2586 (#2582, #2583, #2584, #2585), based on the cumulative top commit and targeting master so the Azure Pipelines PR trigger is eligible.

The permanent review surface remains the four focused stacked PRs. This draft will be closed after the cumulative ADO run is inspected.

Validation goals

  • Confirm independent Databricks CPU/GPU impact gating.
  • Confirm ReleaseBranchCompat is no longer silently skipped for a master target.
  • Confirm reduced AzureCLI task count and removal of Style Conda restoration.
  • Exercise the Entra-based ACR cleanup code through repository tests; the scheduled cleanup itself still requires the dedicated ADO service connection.

ranadeepsingh and others added 8 commits July 30, 2026 05:35
## 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 #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: #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 #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: #2565
- Pull request: #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>
## 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: #2578 (comment)
- Stacked PR: #2581
- Prewarm concurrency review: #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>
## 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 #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: #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>
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Hey @ranadeepsingh 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

  • fix: Fix LightGBM crashes with empty partitions
  • feat: Make HTTP on Spark back-offs configurable
  • docs: Update Spark Serving usage
  • build: Add codecov support
  • perf: improve LightGBM memory usage
  • refactor: make python code generation rely on classes
  • style: Remove nulls from CNTKModel
  • test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@codecov-commenter

codecov-commenter commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.25%. Comparing base (7d9fabc) to head (95c5107).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2587      +/-   ##
==========================================
- Coverage   84.79%   84.25%   -0.55%     
==========================================
  Files         334      334              
  Lines       17806    17806              
  Branches     1623     1623              
==========================================
- Hits        15099    15002      -97     
- Misses       2707     2804      +97     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

## 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
ranadeepsingh force-pushed the ci/stack-validation-2586 branch from b4d9266 to 020d96d Compare August 1, 2026 10:50
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

ranadeepsingh and others added 3 commits August 1, 2026 05:05
## 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>
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>
## 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>
@ranadeepsingh
ranadeepsingh force-pushed the ci/stack-validation-2586 branch from 020d96d to 95c5107 Compare August 1, 2026 12:10
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

Validation complete. ADO build 229205405 passed all 67 jobs, including all five CPU Databricks legs, GPU Databricks, and both release-compat jobs. The permanent review surface is stack #2586: #2582#2583#2584#2585.

Closing this temporary master-targeting PR now; it existed only because the real stacked PR bases are not eligible for the pipeline's pr.branches filter.

@ranadeepsingh
ranadeepsingh deleted the ci/stack-validation-2586 branch August 1, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants