Copilot commented on code in PR #7887:
URL: https://github.com/apache/incubator-seata/pull/7887#discussion_r2908811249
##########
discovery/seata-discovery-zk/src/main/java/org/apache/seata/discovery/registry/zk/ZookeeperRegisterServiceImpl.java:
##########
@@ -154,7 +161,7 @@ public void unregister(InetSocketAddress address) throws
Exception {
}
Review Comment:
`unregister()` removes the path from `REGISTERED_PATH_SET`, but leaves the
corresponding entry in `REGISTERED_PATH_DATA_MAP`. Over time (or with many
ephemeral node registrations) this map will grow without bound. Remove the
entry from `REGISTERED_PATH_DATA_MAP` when unregistering (and consider cleaning
it on failed register as well).
##########
script/client/spring/application.properties:
##########
@@ -60,6 +60,25 @@ seata.client.undo.compress.type=zip
seata.client.undo.compress.threshold=64k
seata.client.load-balance.type=XID
seata.client.load-balance.virtual-nodes=10
+
+# Client routing configuration
+client.routing.enabled=false
+client.routing.debug=false
+client.routing.routers=
+
+# Metadata-router configuration examples (using numbered format)
+# Single router: use metadata-router-1
+# client.routing.routers=metadata-router-1
+# client.routing.metadata-router-1.enabled=true
+# client.routing.metadata-router-1.expression=version >= 2.0
+
+# Multiple routers: use metadata-router-1, metadata-router-2, etc.
+# client.routing.routers=metadata-router-1,metadata-router-2
+# client.routing.metadata-router-1.enabled=true
+# client.routing.metadata-router-1.expression=version >= 2.0
+# client.routing.metadata-router-2.enabled=true
+# client.routing.metadata-router-2.expression=env == prod
Review Comment:
These new routing keys are missing the `seata.` prefix (e.g.
`client.routing.enabled`). In Spring Boot, Seata’s
`SpringBootConfigurationProvider` resolves config by looking up
`seata.<rawDataId>` in the Environment, so these properties won’t be picked up.
Rename them to `seata.client.routing.*` to match the rest of this file (and the
YAML example).
##########
seata-spring-autoconfigure/seata-spring-autoconfigure-client/src/main/resources/META-INF/additional-spring-configuration-metadata.json:
##########
@@ -317,6 +317,51 @@
"type": "java.lang.Integer",
"sourceType":
"org.apache.seata.saga.engine.impl.DefaultStateMachineConfig",
"defaultValue": 1800000
+ },
+ {
+ "name": "seata.client.routing.enabled",
+ "type": "java.lang.Boolean",
+ "description": "Whether enable routing feature.",
+ "sourceType":
"org.apache.seata.spring.boot.autoconfigure.properties.client.RoutingProperties",
+ "defaultValue": false
+ },
+ {
+ "name": "seata.client.routing.debug",
+ "type": "java.lang.Boolean",
+ "description": "Whether enable routing debug mode.",
+ "sourceType":
"org.apache.seata.spring.boot.autoconfigure.properties.client.RoutingProperties",
+ "defaultValue": false
+ },
+ {
+ "name": "seata.client.routing.metadata-routers",
+ "type":
"java.util.Map<java.lang.String,org.apache.seata.spring.boot.autoconfigure.properties.client.MetadataRouterConfig>",
+ "description": "Dynamic metadata router configurations. Key: router name
(e.g. metadata-router-1, metadata-router-2, custom-router), Value: router
configuration",
+ "sourceType":
"org.apache.seata.spring.boot.autoconfigure.properties.client.RoutingProperties"
+ },
Review Comment:
`additional-spring-configuration-metadata.json` declares
`seata.client.routing.metadata-routers`, but there is no corresponding
property/field in `RoutingProperties` (the class defines `routers`,
`metadataRouter`, and `numberedMetadataRouters`). This will mislead IDE
auto-completion and users; either align the metadata name/type with the actual
property model or remove the unused entry.
##########
discovery/seata-discovery-core/src/main/java/org/apache/seata/discovery/routing/router/MetadataRouter.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.discovery.routing.router;
+
+import org.apache.seata.common.ConfigurationKeys;
+import org.apache.seata.common.metadata.ServiceInstance;
+import org.apache.seata.config.Configuration;
+import org.apache.seata.config.ConfigurationFactory;
+import org.apache.seata.discovery.routing.RoutingContext;
+import org.apache.seata.discovery.routing.expression.ConditionMatcher;
+import org.apache.seata.discovery.routing.expression.ExpressionParser;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Metadata router
+ *
+ * Supports three modes:
+ * 1. Single expression: version >= 2.0
+ * 2. OR logic expression: (version >= 2.0) || (env == dev) || (region ==
cn-bj)
+ * 3. AND logic expression: (version >= 2.0) && (env == prod) && (region ==
cn-bj)
+ *
+ * Note: Mixed AND/OR logic is not supported, use multiple MetadataRouters for
complex logic
+ */
+public class MetadataRouter extends AbstractStateRouter<ServiceInstance> {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(MetadataRouter.class);
+ private final Configuration fileConfig =
ConfigurationFactory.CURRENT_FILE_INSTANCE;
+
+ private volatile String expression;
+
+ /**
+ * Constructor with router name
+ * @param routerName router name (e.g., metadata-router-1,
metadata-router-2)
+ */
+ public MetadataRouter(String routerName) {
+ super(routerName);
+ this.expression =
fileConfig.getConfig(ConfigurationKeys.CLIENT_ROUTING_PREFIX + routerName +
".expression");
+ }
+
+ @Override
+ protected List<ServiceInstance> doRoute(List<ServiceInstance> servers,
RoutingContext ctx) {
+ // Create a local copy to ensure consistency during method execution
+ String currentExpression = this.expression;
+
+ // If expression is empty or contains only spaces, return original
server list directly
+ if (currentExpression == null || currentExpression.trim().isEmpty()) {
+ LOGGER.info("The expression is empty, so return original server
list directly.");
+ return servers;
Review Comment:
`doRoute()` logs at INFO every time the expression is blank (`"The
expression is empty..."`). Since routing runs on each client selection, this
can flood logs in normal operation (especially when routing is enabled but a
router is temporarily unconfigured). Consider downgrading to DEBUG (or logging
once during router initialization) to avoid high-volume INFO logs.
--
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]
