fix: propagate LightGBM iteration failures - #2695
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey Rana Singh (@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. |
|
/azp run |
|
/review |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The behavior change is narrowly scoped, aligns with the stated defect/acceptance criteria, and is covered by a targeted regression test asserting exception identity and state invariants.
Review tier: Lite
Findings: None
What changed in this PR
This PR fixes LightGBM training error-handling so that native iteration failures are logged and propagated, allowing Spark to fail the task/job rather than returning a partial model and incorrectly marking training as completed.
Changes:
- Update
TrainUtils.updateOneIterationto log iteration failures and rethrow the original exception instead of converting failures into “early termination”. - Add a regression test that injects a failing
LightGBMBoosterand asserts the same exception instance propagates andstate.isFinishedremainsfalse.
| File | Description |
|---|---|
| lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/TrainUtils.scala | Changes iteration failure handling to error-log and rethrow, preserving correct task/job failure semantics. |
| lightgbm/src/test/scala/com/microsoft/azure/synapse/ml/lightgbm/split1/TrainUtilsSuite.scala | Adds a deterministic regression test covering exception propagation and training-state completion behavior. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
## Summary Stop treating native LightGBM iteration exceptions as successful early completion. Log and rethrow the original failure so Spark fails the task instead of constructing a partial model, with a deterministic regression test that preserves the exception and unfinished state. ## Prompting Intent Investigate the value of closed PR microsoft#2684 and the linked distributed LightGBM issue family, then implement evidence-backed actionable work in an isolated worktree and validate it locally and on Microsoft Fabric. ## Linked Sources - Closed contribution: microsoft#2684 - Historical multiclass report: microsoft#569 - Historical binary socket report: microsoft#728 - Retry diagnostics fix: microsoft#2612 ## Rationale Normal LightGBM completion is already returned by the native update call. An exception represents failure, not early stopping. Rethrowing the same object preserves the native cause and lets Spark apply task and job failure semantics; swallowing it can return an incomplete model that appears valid. This deliberately does not claim to repair every network or executor failure in the broader issue family. Co-authored-by: Copilot <[email protected]>
135078f to
0df0110
Compare
|
/azp run |
|
/review |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/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 #2695 +/- ##
=======================================
Coverage 87.41% 87.41%
=======================================
Files 341 341
Lines 20928 20926 -2
Branches 2217 2218 +1
=======================================
- Hits 18294 18293 -1
+ Misses 2634 2633 -1
🚀 New features to boost your workflow:
|
Related issues and prior work
Supersedes #2684, which was closed because its author could not sign the CLA.
Related to #569 and #728.
This fixes the error-handling defect exposed by those reports. It does not claim
to fix every executor, native-library, resource, or network failure that can
produce a LightGBM socket error.
What changed
TrainUtils.updateOneIterationpreviously caught a native iteration exception,logged it as early termination, set
state.isFinished = true, and allowed modelconstruction to continue.
The updated behavior:
The regression test uses a booster that fails during its native update. Before
the fix, the test failed because no exception escaped. After the fix, it verifies
that the same exception object propagates and training is not marked complete.
Validation
sbt scalastyle "Test / scalastyle" compile test:compilesbt "lightgbm/testOnly com.microsoft.azure.synapse.ml.lightgbm.split1.TrainUtilsSuite"lightgbm-streamingscenario using jars built from this branch:0df0110857334c4443be71b174a82bcbc2e8ca0b3.5.5.5.4.20260807.1lightgbmlib-3.3.510, and LightGBM jar provenance verifiedapplication_1788541423736_0001Repository-wide Black currently reports 11 pre-existing formatting differences
in unchanged Python files. This PR does not modify Python.
Broader LightGBM issue assessment
The customer-reported instability is genuine, but
Connection refusedandnative socket code 104 are usually secondary symptoms. Current
masteralreadycontains fixes for worker-port reservation, driver socket cleanup, retry
diagnostics, barrier topology handling, IPv6 endpoints, and validation-data
transfer.
Separate evidence points to a possible native
lightgbmlib-3.3.510distributedcategorical crash family in #2302, #2438, #2467, and #2674. That needs its own
multi-executor native regression and should not be hidden inside this
error-propagation fix.