strongduanmu commented on PR #39624:
URL: https://github.com/apache/shardingsphere/pull/39624#issuecomment-5450088232
• ### Result
Review Result: Mergeable
At the current PR head cb9193c, ContainerComposerRegistry now closes
expectedDataSourceMap in the shared cleanup path. Based on the public code
evidence, I did not find any new behavioral regression, lifecycle conflict, or
contract
issue.
### Evidence
-
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/container/compose/ContainerComposerRegistry.java:90,
:96, :97, :112 now route both actual and expected data sources through the
same closeDataSourceMap(...) cleanup
path, so the change is confined to resource release.
-
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/container/compose/mode/StandaloneContainerComposer.java:77,
:82 and
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/container/compose/mode/
ClusterContainerComposer.java:88, :93 show that the registry is the
common owner-facing entry point for both actual and expected data source maps,
so the new cleanup step matches the existing ownership model.
-
test/e2e/env/src/test/java/org/apache/shardingsphere/test/e2e/env/container/storage/type/DockerStorageContainer.java:163,
:165, :166 and
test/e2e/env/src/test/java/org/apache/shardingsphere/test/e2e/env/container/storage/type/
NativeStorageContainer.java:76, :77, :133, :137 show that
actualDataSourceMap and expectedDataSourceMap create their own access data
source instances. Closing the expected map therefore does not hand the same map
instance to two
owners for duplicate cleanup.
-
test/e2e/env/src/test/java/org/apache/shardingsphere/test/e2e/env/container/E2EContainers.java:128,
:130, :131, :132 only closes containers and the network, and does not close
these JDBC DataSource pools. This PR fills a real gap in
the existing cleanup chain.
### Coverage
- Reviewed head: cb9193cc09d4d5327496a9faa1b456855a23148c against base
63ab282c627d6262059276aa03532b6f3abaca56 on August 28, 2026.
- Authoritative scope: GitHub pulls/39624/files returned 1 changed file,
and all of it was reviewed.
- Behavior clusters: 1 cluster, the resource lifecycle cleanup in
ContainerComposerRegistry.close().
- Discovery lenses completed: Root Cause and Behavior, Blast Radius and
Contracts, and Tests, Runtime, and Operations.
- Public comments/reviews: there were no public issue comments, reviews,
or review comments to revisit.
- Unresolved gaps: there is no unresolved evidence gap that could change
the blocker set. The PR description still contains the placeholder Fixes
#ISSUSE_ID, but the code-correctness conclusion does not depend on issue
context.
- CI scope: this result covers code correctness only. CI and GitHub
Actions were not reviewed.
--
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]
