yaooqinn commented on PR #12147:
URL: https://github.com/apache/gluten/pull/12147#issuecomment-4551105040
Pushed `da460551e7` — changed the fix strategy after CI surfaced an existing
contract I had missed locally.
**What the previous push got wrong**
The previous commit removed the `input.available() > 0` guard entirely and
read the trailing `hasStats` / `hasSchema` booleans directly. My reasoning was
that Spark cached blocks live within a single application instance, so the
trailing markers are always present and the V1-wire compat path was a phantom.
CI on the previous push (commit `8c362609e9`) revealed
`ColumnarCachedBatchKryoSuite#"V1 wire (no trailing hasStats/hasSchema
booleans) reads as stats=null/schema=null"` — an existing test that locks
"absent trailing must read as null, not throw" as a backward-compat contract.
My phantom-V1 argument was therefore wrong: the contract is real and the test
is the SSOT.
**What this push does**
Replace the `available()` probe with a narrow `try { input.readBoolean() }
catch { case e: KryoException if isBufferUnderflow(e) => false }`. The catch is
filtered on a message-prefix match of `"Buffer underflow"` so genuine
corruption (including `ClassNotFoundException` wrapped during
`readClassAndObject`) is never swallowed.
Net effect:
- V1 wire (no trailing booleans) → `readBoolean()` underflows → caught →
reads as null. Existing contract preserved.
- V2 wire under chunked-fill (BufferedInputStream / network spill returning
`available() == 0` mid-stream) → `readBoolean()` succeeds because Kryo's
`Input.fill()` refills from the underlying stream without consulting
`available()`. The false-EOF probe is gone.
The length-bound `require(... <= maxLen ...)` from `491070bf34` is unchanged.
**Verification**
Local on `-Pspark-4.1 -Pscala-2.13`:
- `ColumnarCachedBatchKryoBoundaryProbeBugSuite` (new, chunked-fill probe) —
1/1 pass
- `ColumnarCachedBatchKryoSuite` (existing, V1 wire silent-null) — 6/6 pass
- Affected siblings (`Framed`, `BuildFilter`, `StatsBlob`,
`ShipBlockerMarshal`, `IntFamilyMarshal`) — 21/21 pass
Sorry for the churn.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
