fix: scale LightGBM validation data transfer

[Rana Singh (ranadeepsingh)]

What this fixes

Closes #2294.

Large sparse validation sets supplied through validationIndicatorCol were preprocessed with collect() and broadcast as Array[Row], making Spark return the complete validation set to the driver and causing driver result-size/heap failures.

Design

• Materialize every preprocessed validation row to a driver-local spool through DataFrame.foreachPartition and bounded 64 KiB driver copy buffers.
• Preserve partition ordering and exact rows; no sampling or validation sharding is introduced.
• Broadcast only an authenticated, time-bounded stream descriptor.
• Stream the complete spool to every executor/task that creates a native LightGBM validation Dataset in both streaming and bulk transfer modes.
• Bound concurrent ingest/serve transfers and clean sockets, attempt files, spool directories, and iterator resources on success and failure.
• Accept the first complete Spark attempt for each partition so retries/speculation cannot overwrite committed partition data.

LightGBM 3.3.5 calculates validation metrics locally, including non-decomposable metrics such as AUC. Each active native worker therefore still receives the complete validation set; changing that would alter validation and early-stopping semantics.

Resource lifecycle

• close() is the single bounded terminal operation: it stops acceptance, closes active clients, terminates the fixed executor, surfaces serving failures, and deletes the spool only after confirmed termination.
• Close clients before executor shutdown, so stalled socket writes are forcibly unblocked; a nonterminating executor fails explicitly and leaves the spool intact.
• Keep spool ownership exception-safe across ingest bind/executor failure, serving bind/executor failure, and serving start failure.
• Treat EOF, socket disconnects, authentication rejection, and accept polling timeouts as expected cancellation/idle behavior while preserving genuine I/O, spool-read, and cleanup failures.
• Bound copy buffers to 64 KiB, concurrent transfers to eight, and ingest/serve thread pools to nine daemon threads each; no per-row or per-transfer result collection is retained.

Compatibility

• No public estimator parameters, JVM method signatures, or serialized parameter shapes change.
• TrainingContext.validationData retains its existing Option[Broadcast[Array[Row]]] type and constructor position.
• Legacy broadcast arrays remain readable; the new broadcast contains only a small versioned descriptor.
• Copy, estimator/model persistence, transform schema, sparse vectors, and both transfer modes are covered.
• Null validation indicators now fail explicitly instead of being silently excluded by SQL null filtering.

Validation

• SynapseML local setup doctor: JDK 11 available; Spark 3.5 / Scala 2.12 confirmed. WSL cannot parse the Windows-path worktree .git indirection, but Windows Git confirms branch/base state.
• lightgbm/compile: passed.
• lightgbm/Test/compile: passed.
• lightgbm/scalastyle: passed with 0 findings.
• lightgbm/Test/scalastyle: passed with 0 findings.
• python3 -m black --check --extend-exclude docs/ .: passed, 194 files unchanged.
• ValidationDataServerLifecycleSuite: 28/28 passed; LightGBMValidationDataSuite: 4/4 passed.
  • Deterministic ingest/serving socket, executor-construction, and server-start failure cleanup.
  • Stalled output client forced closed before executor termination/spool deletion.
  • Cancelled speculative client and idle accept timeout do not poison a later successful transfer.
  • Genuine serving I/O failure remains visible.
  • Nontermination fails explicitly and preserves the spool.
  • 256 rows, 4,096-dimensional sparse vectors with 1,024 deterministic nonzeros, 64 validation rows, four partitions.
  • spark.driver.maxResultSize=128k; a temporary legacy-collect replay failed deterministically at 156.4 KiB for one task and 626.4 KiB across four tasks.
  • Streaming estimator fit, schema, copy, estimator/model persistence, scoring, bulk mode, null validation indicators, and cleanup.
• Confirmed the old validation collect()/large-row broadcast path is absent.
• Existing VerifyLightGBMClassifierStream validation test remains unavailable because au3_25000.csv is not present in this worktree.
• Code generation was not rerun because no wrapper or Python API surface changed.

Merge coordination

PR #2664 overlaps StreamingPartitionTask.scala with #2662. Merge #2662 first, then rebase #2664 onto the updated master. During that rebase, retain both #2662's native LightGBM Dataset failure cleanup and #2664's closeable validation-row iterator handling.

Residual risk

• Exact semantics necessarily retain O(validation data) driver disk usage, full validation network transfer to each active native worker, and a complete native validation Dataset per worker.
• This was validated locally, not at Fabric production scale or against a modified native LightGBM implementation.
• Exact-head Azure validation is requested after the final review; merge readiness remains pending until it completes.

Exact-head review follow-up

Copilot reviewed head 3eaca963317d214c7af6c9f05aa8ee1c01832bc9 and reported two active findings plus one suppressed executor-read finding. Head db086b50f6a42007d6004437a4f27449942c6295 addresses all three:

• Both socket ingest and executor-read paths reject negative row lengths other than the -1 end marker with a clear IOException before copying or allocating.
• No arbitrary upper bound was added because there is no existing serialized-row frame-size invariant; imposing one could reject valid large sparse rows and change supported semantics.
• Broadcast destruction and validation-server close use the existing primary-preserving cleanup pattern, so training or await() failures remain primary and cleanup failures are suppressed.
• Deterministic tests cover malformed lengths in both network directions and simultaneous broadcast/server cleanup failures for both training and await() failures.
• Scala 2.12 main/test compile passed on master; the complete PR patch also applied cleanly and main/test compiled on the live spark4.0 Scala 2.13 branch with JDK 17.

New-head socket-setup follow-up

Copilot reviewed db086b50f6a42007d6004437a4f27449942c6295 and surfaced one suppressed socket-setup leak. Head 965f55d6d9f4680b1eddc4fd7872784d171fff2f wraps executor stream authentication/input initialization in failure-preserving socket cleanup, so getOutputStream, token write/flush, configuration, or getInputStream failures cannot leak the connected socket. A deterministic failing-output socket test verifies the original exception remains primary and the socket is closed. Final evidence is 18/18 lifecycle/integration tests, Scala 2.12 main/test compile, full-patch Spark 4.0/Scala 2.13 main/test compile, both scalastyle tasks, and Black.

Timeout and row-count follow-up

Copilot reviewed 965f55d6d9f4680b1eddc4fd7872784d171fff2f and surfaced two suppressed clarity findings. Head bcae907868ee379c811c670b18a0c36b9bd5ed5c uses an explicit seconds-to-milliseconds conversion independent of accept polling, and validates the downstream native Int row-count range with a targeted IllegalArgumentException instead of exposing ArithmeticException. Deterministic tests cover both. Final Scala 2.12 validation passes 20/20 lifecycle/integration tests, main/test compile, both scalastyle tasks, and Black.

Fractional deadline and iterator cleanup follow-up

Head 3a36dee3bab542510b0ba990a5a18dd814bc57c9 uses millisecond-precise ingest deadlines and primary-preserving iterator cleanup in streaming and bulk consumers. The active thread was replied to and resolved; both suppressed findings are covered by a deterministic processing-plus-close failure test. Scala 2.12 main/test compile, both scalastyle tasks, and 22/22 tests pass. The full patch compiled on Spark 4.0/Scala 2.13 at the preceding head; these final changes use APIs common to both branches.

Maintainability audit on exact head

Head 23c503f71b07681ad318c46f5ed1b6f255d158bf completes the adversarial simplicity audit:

• Removed the separate server await() phase, serving transfer Future queue, ingest Future retention, per-partition row-count map, completed-partition set, and brittle executor/socket test hooks.
• Replaced ingest result retention with a fixed-memory semaphore barrier, row totals with AtomicLong, and completion with a range-validated AtomicInteger.
• Preserved primary failures while retaining close failures as suppressed exceptions, including simultaneous stream/input/socket close failures.
• Rejected unsupported descriptor versions and invalid partition IDs explicitly while preserving legacy row broadcasts and all public JVM/serialized shapes.
• Kept unavoidable costs explicit: O(validation data) driver disk, one complete network transfer per native Dataset owner, and native complete validation data per worker. No arbitrary row cap, sampling, or sharding was introduced.

Validation on this exact diff:

• Spark 3.5 / Scala 2.12 / JDK 11: main/test compile, 27/27 lifecycle tests, 3/3 public estimator tests, and both scalastyle tasks passed.
• Spark 4.1 / Scala 2.13 / JDK 17: the full patch applied to live upstream/spark4.1; main/test compile, 27/27 lifecycle tests, 3/3 public estimator tests, and both scalastyle tasks passed.
• Black and git diff --check passed; codegen remains unaffected because no wrapper or Python API changed.
• A separate combined replay of PR fix: clean up failed LightGBM streaming datasets #2662 plus the intended conflict resolution passed main/test compile, StreamingDatasetLifecycleSuite (1/1), and LightGBMValidationDataSuite (3/3).

Merge order remains #2662 first, then rebase #2664. The rebase must preserve #2662's owned native Dataset cleanup around #2664's streamed validation row source in StreamingPartitionTask.initializeValidationDataset.

Exact-head backlog follow-up

Copilot reviewed 23c503f71b07681ad318c46f5ed1b6f255d158bf and identified the fixed 50-connection ingest accept backlog as a concurrency bottleneck for validation DataFrames with more than 50 partitions. Head 303fb964888c7e4b5f4ba6282eb7babfc9fa2832 passes the exact validation partition count to the listener; the socket implementation still clamps small values to the existing default and the operating system retains its own maximum.

A deterministic regression captured 50 before the fix for 73 partitions and 73 after it. Spark 3.5/Scala 2.12 and Spark 4.1/Scala 2.13 main/test compile, 27/27 lifecycle tests, 3/3 public estimator tests, both scalastyle tasks, and Black passed. The review thread was replied to and resolved.

Suppressed wide-row null-check follow-up

The exact-head review of 303fb964888c7e4b5f4ba6282eb7babfc9fa2832 suppressed one previously missed finding: null-indicator detection returned a complete filtered row to the driver. Head b469be2b811ceb98fdbdcf995a2bec51602c5db8 selects only the boolean indicator before head(1), preserving explicit null rejection without materializing labels or feature vectors.

The public estimator regression now uses deterministic one-megabyte dense feature vectors under spark.driver.maxResultSize=256k; it fails before the projection and returns the intended IllegalArgumentException after it. Spark 3.5/Scala 2.12 and Spark 4.1/Scala 2.13 main/test compile, 27/27 lifecycle tests, 3/3 public estimator tests, both scalastyle tasks, and Black passed.

Suppressed serializer-order follow-up

The exact-head review of b469be2b811ceb98fdbdcf995a2bec51602c5db8 suppressed one previously missed iterator-construction leak: the executor opened its validation socket/input before acquiring a Spark serializer instance. Head 00330a487b2955f31afdb05bdef7967c58b72444 acquires the serializer first, before any network resource exists.

A deterministic ephemeral-listener regression injects a serializer-construction failure and proves the original exception is preserved with no connection attempt. Spark 3.5/Scala 2.12 and Spark 4.1/Scala 2.13 main/test compile, 27/27 lifecycle tests, 3/3 public estimator tests, both scalastyle tasks, and Black passed.

Suppressed subsecond ingest-timeout follow-up

The exact-head review of 00330a487b2955f31afdb05bdef7967c58b72444 suppressed one previously missed timeout mismatch: ingest replaced every configured accept timeout with a fixed one-second poll. Head 864f09efc1d56983b4286750dcc73147a90661e1 uses the smaller of the configured timeout and one second, so polling remains efficient without delaying subsecond deadlines.

A deterministic construction-path regression observes the listener timeout at 250 milliseconds and preserves socket/spool cleanup. Spark 3.5/Scala 2.12 and Spark 4.1/Scala 2.13 main/test compile, 27/27 lifecycle tests, 3/3 public estimator tests, both scalastyle tasks, and Black passed.

Azure coverage-timeout follow-up

Azure build 231779254 validated head 864f09efc1d56983b4286750dcc73147a90661e1 under scoverage. ValidationDataServerLifecycleSuite completed in about 14 seconds, but the original 16,000-row estimator regression consumed each 20-minute test timeout and caused both 30-minute Unit Test attempts to exit 124 before the 80-minute lightgbm1 job cap.

Head 1db7714508886e6cebe578b176bf28913d5e3462 changes only LightGBMValidationDataSuite: 256 rows, four tasks, 4,096 features, 1,024 deterministic nonzeros, eight reference-sample rows, and a 128 KiB driver result limit. A temporary replay of the legacy collect path failed at 156.4 KiB for one task (626.4 KiB total), while the streamed public estimator path passed.

Validation:

• Exact coverage-mode lightgbm1 package command exercised the resized suite in 31.169 seconds and the lifecycle suite in 78.908 seconds; the overall local package command exposed 19 unrelated pre-existing split1 failures.
• The coverage-mode command restricted to those two affected suites passed.
• JDK 11 main/test compile and both scalastyle tasks passed.
• Spark 4.1 / Scala 2.13 main/test compile, both scalastyle tasks, 27/27 lifecycle tests, and 3/3 estimator tests passed.
• No production logic changed.

Exact-head cancellation, spool fallback, and bulk-helper follow-up

Heads 61013c25d88b28b6ca67d89600ec331e92a89e36, 263da875915bd7e18dbb9c70e12ffd78044e1635, and 85ec1fc111e6676ea7f29fd57cc5632f5ef41610 close the remaining coverage and exact-head review findings:

• The stalled-client test double treats executor interruption as expected only after its tracked socket is closed, eliminating the uncaught coverage-thread error while retaining unexpected early interruptions.
• Default spool creation still prefers user.dir, but atomically falls back to the JVM temporary directory when that parent is unusable; dual failures preserve the preferred error as suppressed.
• Bulk single-dataset helpers no longer download the complete validation spool. Only the active task per executor reads it; helpers decrement the existing validation synchronization latch. Bulk per-task Dataset mode remains unchanged and transfers the complete validation set to every native Dataset owner.
• The short accept-poll timeout test now uses a separate five-second client read timeout, preventing coverage overhead from creating a false socket timeout.

Exact-head validation:

• Spark 3.5 / Scala 2.12 / JDK 11: main/test compile and both scalastyle tasks passed; 28/28 lifecycle tests and 4/4 public estimator tests passed.
• Coverage mode: the combined lifecycle/integration selection passed 32/32 with no uncaught interruption.
• Spark 4.1 / Scala 2.13 / JDK 17: full-patch main/test compile, both scalastyle tasks, and 4/4 public estimator tests passed; the exact fallback and stalled-client tests also passed.
• Copilot reviewed exact head 85ec1fc111e6676ea7f29fd57cc5632f5ef41610 with zero generated or suppressed findings; all review threads are resolved.

Merge order remains #2662 first, then rebase #2664, preserving #2662's native Dataset ownership cleanup in the overlapping StreamingPartitionTask.scala path.

Rebase onto current master

Head 395bd9aa2c899a836bc5cbcb762b9fec47b1ef79 is rebased onto origin/master 9f152c2ef78d9975ff4f855e1d05b85cb358563e.

• The rebase completed without conflicts.
• git range-diff matched all 20 commits one-for-one, and the six intended LightGBM source/test files are byte-identical to pre-rebase head 85ec1fc111e6676ea7f29fd57cc5632f5ef41610.
• JDK 11 validation passed: lightgbm/Test/compile, 4/4 LightGBMValidationDataSuite tests, and the filtered stalled-serving-client lifecycle test.
• git diff --check passed and the worktree was clean.
• origin/master was fetched again immediately before the lease-protected force push and remained at the recorded target SHA.

PR #2662 remains open and was neither merged nor incorporated.

[azure-pipelines]

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

[github-actions]

Hey Rana Singh (@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.

[Rana Singh (ranadeepsingh)]

Lifecycle follow-up pushed at 3eaca96. Active ingest/serve sockets are now closed before executor shutdown; spool deletion requires confirmed termination; construction ownership is covered through bind, executor, and start failures; cancellation/accept timeouts remain nonterminal while real I/O failures propagate. JDK 11 compile/test-compile and both scalastyle tasks passed, Black passed, and the two targeted suites passed 13/13. Copilot automated review was requested and polled twice for 10 minutes but no review exists for this head. Azure was intentionally not triggered. Merge #2662 first, then rebase #2664 and preserve both cleanup changes.

[azure-pipelines]

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

[Copilot]

Pull request overview

This PR fixes LightGBM training failures when validationIndicatorCol is used with large/sparse validation sets by eliminating the driver-side collect() + broadcast of all validation rows. Instead, it spools preprocessed validation rows to driver-local disk and streams them to executors/tasks that construct native LightGBM validation Datasets, preserving full validation semantics (each worker still receives the complete validation set).

Changes:

• Add a driver-side ValidationDataServer that ingests per-partition validation rows via sockets into a spool directory, then serves the spool to executors via authenticated streaming.
• Update streaming and bulk partition tasks to consume validation rows via ValidationDataServer.read(...) (iterator + explicit close) and to use the streamed row count.
• Add lifecycle and end-to-end tests covering cleanup, persistence, bulk/stream modes, and null validation indicators.

Show a summary per file

File	Description
lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/ValidationDataServer.scala	New spool + stream server/descriptor format for scalable validation data transfer.
lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/LightGBMBase.scala	Replaces validation collect() broadcast path with server-backed broadcast descriptor and explicit null-indicator rejection.
lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/StreamingPartitionTask.scala	Streams validation rows into the shared streaming validation Dataset and closes the iterator.
lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/BulkPartitionTask.scala	Streams validation rows for bulk-mode aggregation and closes the iterator.
lightgbm/src/test/scala/com/microsoft/azure/synapse/ml/lightgbm/split1/ValidationDataServerLifecycleSuite.scala	New deterministic tests for server lifecycle cleanup and failure handling.
lightgbm/src/test/scala/com/microsoft/azure/synapse/ml/lightgbm/split1/LightGBMValidationDataSuite.scala	New integration tests ensuring validation streaming avoids driver result-size issues and covers persistence/bulk/null-indicator behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/ValidationDataServer.scala:689

• Row lengths come from the network stream; a negative value other than the EndOfStream marker should be rejected explicitly. Without a guard this can throw NegativeArraySizeException (or lead to unexpected allocation behavior) instead of a clear I/O error.

          val length = input.readInt()
          if (length == EndOfStream) {
            close()
            None
          } else {

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

[Rana Singh (ranadeepsingh)]

Suppressed finding from Copilot review 4964793710 is explicitly fixed in db086b5: executor-side validation row lengths now pass through the same protocol validator as ingest, rejecting negative values other than the -1 end marker before allocation. A deterministic malformed executor-stream test covers that exact path. No upper cap was invented because the protocol has no established maximum serialized-row size and a new cap could reject valid sparse rows. Both active threads were also fixed, replied to, and resolved. Local evidence: Scala 2.12 main/test compile; full patch on spark4.0 Scala 2.13 main/test compile; 17/17 lifecycle/integration tests; both scalastyle tasks; Black. Azure was not triggered. #2662 must still merge first, followed by rebasing #2664 while preserving both StreamingPartitionTask cleanup changes.

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/ValidationDataServer.scala:678

• The ValidationDataServer.read(params) iterator can leak an open socket if authentication/stream setup fails before input is initialized (e.g., socket.getOutputStream, writeUTF, or flush throws). Wrap initialization in a try/catch that closes the socket on failure and only keep input as a field.

      private val socket = connect(params.host, params.port, params.timeoutMillis)
      socket.setKeepAlive(true)
      socket.setTcpNoDelay(true)
      socket.setSoTimeout(params.timeoutMillis)
      private val auth = new DataOutputStream(new BufferedOutputStream(socket.getOutputStream))
      auth.writeUTF(params.token)
      auth.flush()
      private val input = new DataInputStream(new BufferedInputStream(socket.getInputStream))

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

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

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

[Rana Singh (ranadeepsingh)]

Addressed the new-head suppressed socket-setup finding in 965f55d. ValidationDataServer.read now closes its connected socket through the existing primary-preserving cleanup helper if socket configuration, authentication output/write/flush, or input-stream construction fails. A deterministic authentication-output failure test proves the original IOException remains primary and the socket is closed. Final validation: 18/18 lifecycle/integration tests; Scala 2.12 main/test compile; the full PR patch applied cleanly and main/test compiled on spark4.0 Scala 2.13/JDK 17; both scalastyle tasks; Black. Azure remains intentionally untriggered. #2662 still merges first, then #2664 rebases while retaining both StreamingPartitionTask cleanup changes.

[Copilot]

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/ValidationDataServer.scala:478

• timeoutMillis is calculated using IngestPollTimeoutMillis as the seconds→milliseconds conversion factor. This couples the socket timeout semantics to the accept-poll constant, so changing the poll interval would silently change all timeouts. Use an explicit seconds→ms conversion instead (e.g., timeoutSeconds * 1000.0).

      val timeoutMillis = (timeoutSeconds * IngestPollTimeoutMillis).toLong
      socket.setSoTimeout(Math.max(IngestPollTimeoutMillis, timeoutMillis).min(Int.MaxValue).toInt)

lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/ValidationDataServer.scala:658

• rowCount converts a Long to Int via Math.toIntExact, which will throw a bare ArithmeticException if validation row count exceeds Int.MaxValue. Since downstream LightGBM APIs require Int row counts anyway, it would be clearer to fail with a targeted IllegalArgumentException explaining the limit.

  def rowCount(data: Broadcast[Array[Row]]): Int = {
    ValidationDataParams.fromBroadcast(data)
      .map(params => Math.toIntExact(params.rowCount))
      .getOrElse(data.value.length)
  }

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

[Rana Singh (ranadeepsingh)]

Addressed both suppressed findings from the 965f55d review in bcae907. Socket timeout conversion now uses an explicit seconds-to-milliseconds constant rather than the accept-poll interval, and validation row counts outside 0..Int.MaxValue fail with a targeted IllegalArgumentException describing the downstream native limit. Deterministic tests cover 2.5 seconds -> 2500 ms and Int overflow. Final Scala 2.12 evidence: main/test compile, both scalastyle tasks, Black, 20/20 lifecycle/integration tests. Azure remains intentionally untriggered; #2662 still merges first, then #2664 rebases preserving both StreamingPartitionTask cleanup changes.

[Copilot]

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/StreamingPartitionTask.scala:165

• rows.close() can throw (socket/input close), which would mask an exception from insertRowsIntoDataset and make failures harder to diagnose. Suppress non-fatal close failures so the primary training/validation error remains primary.

    } finally {
      rows.close()
    }

lightgbm/src/main/scala/com/microsoft/azure/synapse/ml/lightgbm/BulkPartitionTask.scala:53

• rows.close() can throw (socket/input close), which would mask an exception from getChunkedColumns / mergeChunksIntoAggregatedArrays. Suppress non-fatal close failures so the primary error remains primary.

      } finally {
        rows.close()
      }

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

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

[Rana Singh (ranadeepsingh)]

Exact-head Azure validation is queued as build 231826848. ADO trigger metadata confirms reason=pullRequest, refs/pull/2664/merge, and pr.sourceSha=85ec1fc111e6676ea7f29fd57cc5632f5ef41610; status is currently inProgress. Final automated review covers this head with 0 generated and 0 suppressed findings, and there are 0 unresolved review threads. Remaining integration order: merge #2662 first, then rebase #2664 and preserve both overlapping StreamingPartitionTask.scala cleanup paths.

[Rana Singh (ranadeepsingh)]

Rebased exact six-file patch onto origin/master 9f152c2ef78d9975ff4f855e1d05b85cb358563e; new head 395bd9aa2c899a836bc5cbcb762b9fec47b1ef79. No conflicts. All 20 commits matched one-for-one in git range-diff, and all six changed files are identical to pre-rebase head 85ec1fc111e6676ea7f29fd57cc5632f5ef41610. JDK 11 lightgbm/Test/compile, 4/4 estimator tests, the stalled-client lifecycle regression, and git diff --check passed. Master was fetched again immediately before the --force-with-lease push and was unchanged. #2662 remains open and was not incorporated.

[Copilot]

Review details

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

[Rana Singh (ranadeepsingh)]

/azp run

[azure-pipelines]

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

[Rana Singh (ranadeepsingh)]

Rebased-head audit complete: Copilot review at 395bd9aa2c899a836bc5cbcb762b9fec47b1ef79 reviewed all 6 files with 0 generated findings and no suppressed section; readiness reports 0 unresolved threads and 0 suppressed findings for head. New Azure build 231829385 is inProgress, reason=pullRequest, validating refs/pull/2664/merge with pr.sourceSha=395bd9aa2c899a836bc5cbcb762b9fec47b1ef79. Final target recheck remains origin/master 9f152c2ef78d9975ff4f855e1d05b85cb358563e (20 ahead, 0 behind).