LuciferYang opened a new pull request, #12658: URL: https://github.com/apache/gluten/pull/12658
### What changes were proposed in this pull request? `ComponentSuite` registers dummy components into the JVM-global component graph, including a deliberate dependency cycle, and never removes them. Any later suite in the same JVM that calls `Component.sorted()` then fails with `Cycle detected in the component graph: B, D, C`, naming components unrelated to the failing test. Nothing in `gluten-core` calls `sorted()` after `ComponentSuite` today, but ten call sites in main sources reach it, so a suite that boots a `SparkContext` with the Gluten plugin fails as soon as it is ordered after `ComponentSuite`. `Component`'s graph and the `allComponentsLoaded` discovery latch are `object`-level state with no reset, so a suite that registers components cannot clean up after itself. This adds a testing-only reset that empties the graph, clears each component's registration flag so it can register again, and re-arms the latch. `ComponentSuite#afterAll` calls it. `ensureRegistered` now rolls its flag back when `graph.add` throws. A component that never entered the graph is invisible to `Registry#clear`, so without the rollback its flag stays set and it can never register again. The rollback covers only `graph.add`, not `dependencies()`: a component that is already in the graph would then fail its next registration with a misleading "UID already registered". `resetRegisteredForTesting` is `final`, since nine of the eleven in-repo `Component` implementations sit in this package and could otherwise override it to a no-op and silently defeat the clear. Four values survive the reset, and the `clearAllForTesting` scaladoc now says so: `BackendsApiManager.backend`, `GlutenCostModel.costModelRegistry`, the `graphCache` inside `Transition.factory`, and the per-vertex `TransitionGraph.Vertex.initialized` flags all keep what they computed from the pre-clear component set. Rediscovery also constructs fresh instances, so one of those values can end up holding an instance that is no longer the one in the graph. The latch line carries a comment explaining it serves the backend modules, whose classpath carries component files, so it does not read as dead code here. ### How was this patch tested? Added `ComponentGraphResetSuite` with three tests, each covering one part of the reset: - register a cycle, reset, assert the graph is empty; - register a component, reset, register the same instance again, assert it is back in the graph. This fails without the registration-flag reset; - run `ComponentSuite` in-process via `new ComponentSuite().run(None, Args(reporter))` and assert the graph is empty afterwards. That covers the fix's own call site without depending on suite execution order, and fails when `ComponentSuite#afterAll` is removed. Cleanup lives in `afterAll` so a failed assertion cannot leak the cycle the suite registers. I verified each test fails against the corresponding mutant, and that `mvn -pl gluten-core,gluten-substrait test` passes (37 + 51), both in the natural suite order and with `ComponentSuite` forced to run first. Closes #12655 -- 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]
