jackylee-ch commented on code in PR #13150:
URL: https://github.com/apache/gluten/pull/13150#discussion_r4113756493
##########
gluten-core/src/main/scala/org/apache/spark/shuffle/ShuffleManagerRouter.scala:
##########
@@ -111,7 +113,12 @@ private object ShuffleManagerRouter {
def get(shuffleId: Int): ShuffleManager = {
val manager = cache.get(shuffleId)
- assert(manager != null, s"Shuffle manager not registered for shuffle id:
$shuffleId")
+ if (manager == null) {
+ // An assert is a no-op on JVMs launched without -ea (Spark's default
for
+ // executors), which would turn a cache miss into a bare NPE at the
+ // delegation call site instead of this diagnostic.
+ throw new GlutenException(s"Shuffle manager not registered for shuffle
id: $shuffleId")
Review Comment:
Scala `assert` is compile-time `@elidable`, not `-ea`-gated (not elided in
this build) — the old code already threw AssertionError with the id, not a bare
NPE. And Error→Exception flips local/host-local reads (fetchLocalBlocks catches
`case e: Exception`) from a task failure to FetchFailed.
--
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]
