terrymanu commented on PR #38999: URL: https://github.com/apache/shardingsphere/pull/38999#issuecomment-4883764030
### Summary **Review Result: Mergeable** Reason: This is a code-scope-only result for [apache/shardingsphere#38999](https://github.com/apache/shardingsphere/pull/38999); CI was not reviewed by request. The patch consistently removes the ShardingCache feature surface from runtime routing, Java/YAML configuration, rule-item SPI registration, tests, fixtures, and native reachability metadata without leaving supported-source residue or compatibility shims. ### Evidence - Runtime routing now uses the normal sharding route path directly in `features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/route/engine/ShardingSQLRouter.java:49`; there is no remaining cache-enabled branch before route creation. - Java and YAML configuration surfaces no longer expose ShardingCache fields in `features/sharding/api/src/main/java/org/apache/shardingsphere/sharding/api/config/ShardingRuleConfiguration.java:45` and `features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/yaml/config/YamlShardingRuleConfiguration.java:48`. - YAML swapper logic no longer parses or emits ShardingCache in `features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/yaml/swapper/YamlShardingRuleConfigurationSwapper.java:58` and `features/sharding/core/src/main/java/org/apache/shardingsphere/sharding/yaml/swapper/YamlShardingRuleConfigurationSwapper.java:104`. - Rule item SPI registration no longer includes `ShardingCacheChangedProcessor` in `features/sharding/core/src/main/resources/META-INF/services/org.apache.shardingsphere.mode.spi.rule.RuleItemConfigurationChangedProcessor:18`. - Fixture expectations were updated consistently: tuple count drops to 20 in `features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/yaml/ShardingConfigurationYamlRuleNodeTupleSwapperEngineIT.java:38`, and expected YAML config no longer constructs ShardingCache in `features/sharding/core/src/test/java/org/apache/shardingsphere/sharding/yaml/ShardingRuleConfigurationYamlIT.java:52`. - Native reachability metadata no longer lists the removed ShardingCache processor or YAML field in `infra/reachability-metadata/src/main/resources/META-INF/native-image/org.apache.shardingsphere/generated-reachability-metadata/reachability-metadata.json:4648`. ### Review Details - Review Focus: Code Correctness Review. CI not reviewed by request. - Reviewed Scope: PR #38999, latest head `b923beb087fee6b4275cd50ea4195f1cd0f63bd3`, base `7c438cedc33afc67b3127baf02620c7e887c6b1e`, merge-base `7c438cedc33afc67b3127baf02620c7e887c6b1e`. Reviewed all 33 changed files reported by GitHub; local triple-dot file list matched GitHub `/pulls/38999/files`. - Not Reviewed Scope: GitHub Actions/check-runs, full repository build, full GraalVM native-image build, and unrelated modules outside the changed file set. - Verification: - `rg -n "ShardingCache|shardingCache|sharding_cache|YamlShardingCache|CachedShardingSQLRouter|ShardingRouteCache|CacheableShardingAlgorithmChecker|routeCache|writeQueryConnectionLimit|readQueryConnectionLimit|getShardingCache|setShardingCache|isShardingCacheEnabled" . --glob '!target/**' --glob '!RELEASE-NOTES.md'` exited 1, meaning no supported-source residue was found. - `rg -n "com.github.benmanes.caffeine|Caffeine|LoadingCache" features/sharding --glob '!target/**'` exited 1, meaning no sharding-module Caffeine residue was found. - `jq empty infra/reachability-metadata/src/main/resources/META-INF/native-image/org.apache.shardingsphere/generated-reachability-metadata/reachability-metadata.json` exited 0. - `./mvnw checkstyle:check -Pcheck -T1C` exited 0. - `./mvnw -pl features/sharding/api,features/sharding/core,features/sharding/distsql/handler -am -Pcheck -T1C test` exited 0. - `./mvnw -pl infra/reachability-metadata -Pcheck -T1C test` exited 0. - Release Note / User Docs: Active docs/examples/README search found no ShardingCache references. Historical `RELEASE-NOTES.md` still contains the old experimental-history entry and was not changed; no additional user-doc update is required for this code-scope review. -- 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]
