LuciferYang opened a new pull request, #12609: URL: https://github.com/apache/gluten/pull/12609
### What changes were proposed in this pull request? `ShuffleManagerRouter`'s inner `Cache` assumed each shuffleId is stored once, before any `get` or `remove`. That holds on the driver, where `registerShuffle` runs single-threaded from the DAGScheduler, but not on executors, where the cache is filled lazily and concurrently by task threads through `getReader`/`getWriter`. Two problems followed. `store` used `cache.compute` with `assert(m == null)`. When several tasks of the same new shuffleId first touch the cache at once on a multi-core executor, `ConcurrentHashMap.compute` serializes them and the later threads hit that assertion, so those tasks fail with an `AssertionError`. A retry succeeds once the cache is populated, but the failures still surface as flaky tasks. This change makes `store` an idempotent `computeIfAbsent` and takes the manager by name, so the lookup still runs only on a miss. The `has()` check in `ensureShuffleManagerRegistered` is then redundant, so it is removed. `remove` used `assert(manager != null)`. Spark broadcasts `RemoveShuffle` to every executor, so a router that never cached a shuffleId still gets `unregisterShuffle` and hits that assertion. `remove` now returns an `Option`, and `unregisterShuffle` reports that it removed nothing, matching `SortShuffleManager`, which returns a boolean instead of throwing. `Cache.get` still asserts. Every path that reaches it runs on an executor that already registered the shuffleId (as writer via `getWriter`, or as reader via `getReader`, which registers before serving), so it is left unchanged. ### How was this patch tested? Added `ShuffleManagerRouterCacheSuite` with two tests: - `unregisterShuffle` on a shuffleId this router never cached returns `false` instead of throwing. - 4 threads over 200 iterations first-touching the same new shuffleId produce no errors. Both fail on the current code (they hit the two assertions) and pass after the fix. The existing `GlutenShuffleManagerSuite` still passes. Closes #12608 -- 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]
