Skip to content

fix: skip VW barrier execution for single-partition training - #2592

Open
ranadeepsingh wants to merge 1 commit into
microsoft:masterfrom
ranadeepsingh:copilot/ancient-1912-vw-barrier
Open

fix: skip VW barrier execution for single-partition training#2592
ranadeepsingh wants to merge 1 commit into
microsoft:masterfrom
ranadeepsingh:copilot/ancient-1912-vw-barrier

Conversation

@ranadeepsingh

Copy link
Copy Markdown
Collaborator

Summary

  • use Vowpal Wabbit barrier execution only when it is enabled and training has more than one partition
  • preserve the existing multi-partition barrier path and disabled-barrier path
  • add execution-path coverage for enabled single-partition, enabled multi-partition, and disabled multi-partition training

Validation

  • TDD red: the new one-partition test observed a BarrierTaskContext before the source guard
  • vw/testOnly com.microsoft.azure.synapse.ml.vw.VerifyVowpalWabbitClassifier -- -z skips
  • vw/testOnly com.microsoft.azure.synapse.ml.vw.VerifyVowpalWabbitClassifier -- -z libsvm
  • vw/compile and vw/Test/compile
  • scalastyle test:scalastyle

Compatibility

The production change is a source- and binary-compatible execution guard; no public API changes are introduced. This change is scoped only to Vowpal Wabbit and does not alter LightGBM.

Related

Recreates the still-relevant proposal from #1912 on current master without closing or modifying that original PR.

@ranadeepsingh
ranadeepsingh requested a review from eisber as a code owner August 1, 2026 12:18
Copilot AI review requested due to automatic review settings August 1, 2026 12:18
@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

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.

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

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

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 adjusts Vowpal Wabbit training to avoid Spark barrier execution when training runs with a single partition, while preserving the existing behavior for multi-partition training (both barrier-enabled and barrier-disabled). This improves compatibility with single-partition workloads by preventing unnecessary barrier scheduling.

Changes:

  • Guard barrier execution in VowpalWabbitBaseLearner so it only runs when barrier mode is enabled and the training DataFrame has more than one partition.
  • Add execution-path tests that verify barrier usage (enabled multi-partition), non-barrier usage (disabled multi-partition), and skipping barrier usage (enabled single-partition) via a tracking subclass and accumulator.
Show a summary per file
File Description
vw/src/main/scala/com/microsoft/azure/synapse/ml/vw/VowpalWabbitBaseLearner.scala Adds a partition-count guard to skip barrier execution for single-partition training.
vw/src/test/scala/com/microsoft/azure/synapse/ml/vw/VerifyVowpalWabbitClassifier.scala Adds tests that assert which execution path was used (barrier vs non-barrier) across partition and configuration combinations.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

@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 78.33%. Comparing base (7d9fabc) to head (71a0815).

❗ There is a different number of reports uploaded between BASE (7d9fabc) and HEAD (71a0815). Click for more details.

HEAD has 10 uploads less than BASE
Flag BASE (7d9fabc) HEAD (71a0815)
53 43
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2592      +/-   ##
==========================================
- Coverage   84.79%   78.33%   -6.46%     
==========================================
  Files         334      334              
  Lines       17806    17806              
  Branches     1623     1608      -15     
==========================================
- Hits        15099    13949    -1150     
- Misses       2707     3857    +1150     

☔ 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
Use barrier execution only when VowpalWabbit training enables it and the prepared dataset has more than one partition. Add Spark-stage execution-path assertions for enabled multi-partition, disabled multi-partition, and enabled single-partition training.

## Prompting Intent
Revalidate the current VowpalWabbitBaseLearner behavior, restore the minimal source-compatible rule proposed by the ancient PR, prove it through TDD and targeted VW validation, and keep the change isolated from LightGBM.

## Linked Sources
- Original proposal: microsoft#1912
- Initial CI failure: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229206650
- Repository review guidance: .github/skills/code-review/SKILL.md
- Local validation guidance: .github/skills/synapseml-local-setup/SKILL.md

## Rationale
Keep the public API and multi-partition synchronization behavior unchanged with a short-circuit partition-count guard. Observe Spark stage metadata rather than adding a test-only PipelineStage subclass, and use a marker job to drain asynchronous listener events deterministically; this proves the selected execution path without entering global stage discovery.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranadeepsingh
ranadeepsingh force-pushed the copilot/ancient-1912-vw-barrier branch from 91c7c96 to 71a0815 Compare August 1, 2026 13:51
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

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

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.

3 participants