Skip to content

ci: guard Databricks T4 GPU pool migration AB#5478524 - #2579

Open
ranadeepsingh wants to merge 2 commits into
microsoft:masterfrom
ranadeepsingh:rana/enforce-t4-gpu-pool
Open

ci: guard Databricks T4 GPU pool migration AB#5478524#2579
ranadeepsingh wants to merge 2 commits into
microsoft:masterfrom
ranadeepsingh:rana/enforce-t4-gpu-pool

Conversation

@ranadeepsingh

Copy link
Copy Markdown
Collaborator

Related Issues/PRs

What changes are proposed in this pull request?

Require the stable SynapseML Databricks GPU pool, synapseml-build-14.3-gpu, to use
Standard_NC16as_T4_v3 before a GPU test cluster is created.

  • Resolve the pool by its stable name and validate its node_type_id.
  • Fail with a clear error if the pool is missing or points to retired NCv3 hardware.
  • Add focused coverage for the migrated T4 pool and an NCv3 regression case.

How the pipeline migration works

  1. The Databricks E2E tests already look up the GPU pool by the stable name
    synapseml-build-14.3-gpu; the pipeline does not hardcode a pool ID or VM name.
  2. The Azure Databricks infrastructure cutover assigned that stable name to T4 pool
    0725-023302-axle5-pool-1a0ah38i.
  3. Existing pipeline runs therefore switched to T4 automatically without a YAML or service
    connection change.
  4. The retired NCv3 pool was deleted through Databricks, which automatically removed all
    Standard_NC6s_v3 VMs, disks, and NICs.
  5. This PR makes that infrastructure contract explicit so a future pool-name reassignment cannot
    silently return CI to NCv3 or another unexpected SKU.

The active pool keeps one warm T4 worker, allows at most two workers, uses the CPU pool for the
driver, and retains the existing two-worker-to-one-worker stockout fallback from #2573.

How is this patch tested?

  • I have written tests and confirmed the proposed change works.

  • sbt "core/testOnly com.microsoft.azure.synapse.ml.nbtest.DatabricksUtilitiesSuite"

  • sbt core/scalastyle "core/Test/scalastyle"

  • black --check --extend-exclude 'docs/' .

  • Live executor probe returned Tesla T4, 15360 MiB.

  • All three Databricks GPU notebooks passed on the T4 worker:

Does this PR change any dependencies?

  • No. You can skip this section.
  • Yes. Make sure the dependencies are resolved correctly, and list changes here.

Does this PR add a new feature? If so, have you added samples on website?

  • No. You can skip this section.
  • Yes. Make sure you have added samples following below steps.

Require the stable SynapseML GPU pool to resolve to Standard_NC16as_T4_v3 before creating a Databricks cluster. This prevents the pipeline from silently returning to retired NCv3 hardware.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 29, 2026 12:21
@azure-pipelines

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

@github-actions

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the Databricks GPU CI contract by ensuring the stable SynapseML GPU instance pool name (synapseml-build-14.3-gpu) resolves to the expected migrated T4 node type (Standard_NC16as_T4_v3) before creating GPU test clusters, and adds targeted regression coverage.

Changes:

  • Add a fixed expected GPU pool node type constant and require the resolved pool to match it.
  • Refactor pool ID lookup into a reusable selectPoolId helper with clearer failure behavior.
  • Add a focused test covering the migrated T4 pool and a retired NCv3 regression case.
Show a summary per file
File Description
core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/DatabricksUtilities.scala Enforces expected GPU pool node type when resolving the stable GPU pool ID; introduces selectPoolId helper.
core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/DatabricksUtilitiesSuite.scala Adds tests validating correct pool selection for T4 and failure for NCv3 node type.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Low

Match both the stable pool name and expected T4 node type so duplicate Databricks pool names cannot make selection depend on API ordering.

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:
Successfully started running 1 pipeline(s).

@codecov-commenter

codecov-commenter commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.79%. Comparing base (b350135) to head (1a76dc0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2579   +/-   ##
=======================================
  Coverage   84.79%   84.79%           
=======================================
  Files         334      334           
  Lines       17801    17801           
  Branches     1619     1619           
=======================================
  Hits        15094    15094           
  Misses       2707     2707           

☔ 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.

@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

/azp run

@azure-pipelines

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

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

The Databricks GPU job passes. The remaining six Python failures also reproduce on master and are caused by the shared cached MLflow/protobuf environment. The isolated fix is tracked in #2580 (AB#5480878); once that lands, this PR can be refreshed and rerun.

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

Hotfix #2580 has now passed the full Azure pipeline in build 228877170, including all six previously failing Python jobs and the Databricks GPU job. It is awaiting approval/merge; after it lands, this branch can be refreshed and /azp run rerun against the repaired environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants