LuciferYang opened a new issue, #12608:
URL: https://github.com/apache/gluten/issues/12608
### Backend
VL (Velox) — the logic is backend-agnostic and lives in `gluten-core`.
### Bug description
`ShuffleManagerRouter`'s inner `Cache` assumes a single-coordinator
lifecycle: each shuffleId is `store`d exactly once, before any `get`/`remove`.
That holds on the driver (`registerShuffle` is called single-threaded by the
DAGScheduler), but **not** on executors, where the per-executor router cache is
populated lazily by task threads via `getReader`/`getWriter` →
`ensureShuffleManagerRegistered`. Two observable problems follow.
**1. Concurrent first-touch race (primary).**
On a multi-core executor, N tasks of the same *new* shuffleId call
`getReader`/`getWriter` concurrently. In `ensureShuffleManagerRegistered`, the
`has()` check and the `store()` call are not atomic, and `store()` uses
`cache.compute` with `assert(m == null)`. `ConcurrentHashMap.compute` holds a
per-bin lock, which makes the failure **deterministic for the losers**: exactly
one thread installs the manager, and every other thread's remapping function
observes `m != null` → the `assert` fires → `AssertionError` propagates out of
`getReader`/`getWriter` → those tasks are marked FAILED. It is
retry-recoverable (by retry time the cache is populated), but it produces
intermittent, timing-dependent spurious task failures that count toward
`spark.task.maxFailures` and add latency; a wide new-shuffle stage produces a
burst.
**2. `unregisterShuffle` asserts for an uncached shuffleId.**
Spark broadcasts `RemoveShuffle` to **every** executor
(`BlockManagerMasterEndpoint.removeShuffle` → each executor's
`BlockManagerStorageEndpoint`). An executor that never participated in a given
shuffle has no entry in its router cache, so `Cache.remove`'s `assert(manager
!= null)` fires. The `AssertionError` is caught by `doAsync`'s Future (no
crash, no task/job failure), but every shuffle cleanup emits an ERROR-level
stack trace on each non-participating executor plus a WARN on the driver, and
`unregisterShuffle` returns a failed RPC instead of a boolean — polluting error
logs/metrics and potentially masking genuine removal errors.
Note: Scala's `Predef.assert` is an ordinary method call that is **not**
gated by the JVM `-ea` flag, and the Gluten build sets no `-Xelide-below`, so
these assertions fire in both test and shipped builds.
`GlutenShuffleManager` is currently annotated `@Experimental`.
A third manifestation of the same root cause is latent: `Cache.get` (reached
via `BlockResolver`) also asserts, but every path that reaches it is on an
already-registered executor, so it is not a reachable crash on a supported
configuration and is out of scope here.
### Gluten version
main (1.8.0-SNAPSHOT)
### Spark version
Version-agnostic (applies to spark-3.3 / 3.4 / 3.5 / 4.0 / 4.1).
### Spark configurations
- `spark.shuffle.manager=org.apache.spark.shuffle.GlutenShuffleManager`
- Problem 1 requires `spark.executor.cores > 1` (multi-core executor).
- Problem 2 requires a multi-executor cluster.
### System information
N/A — logic issue in
`gluten-core/src/main/scala/org/apache/spark/shuffle/ShuffleManagerRouter.scala`,
independent of OS/hardware.
### Relevant logs
```text
java.lang.AssertionError: assertion failed: Shuffle manager was already
cached for shuffle id: <id>
at
org.apache.spark.shuffle.ShuffleManagerRouter$Cache.store(ShuffleManagerRouter.scala:109)
at
org.apache.spark.shuffle.ShuffleManagerRouter.ensureShuffleManagerRegistered(ShuffleManagerRouter.scala:95)
at org.apache.spark.shuffle.ShuffleManagerRouter.getReader/getWriter(...)
java.lang.AssertionError: assertion failed: Shuffle manager not registered
for shuffle id: <id>
at
org.apache.spark.shuffle.ShuffleManagerRouter$Cache.remove(ShuffleManagerRouter.scala:122)
at
org.apache.spark.shuffle.ShuffleManagerRouter.unregisterShuffle(ShuffleManagerRouter.scala:64)
```
### Fix direction
Make the router `Cache` tolerate the executor lifecycle: `store` idempotent
(`computeIfAbsent`), and `remove`/`unregisterShuffle` handle absence gracefully
(return `Option`/`false`) instead of asserting.
--
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]
