chia7712 commented on code in PR #20938:
URL: https://github.com/apache/kafka/pull/20938#discussion_r2660522820


##########
server/src/test/java/org/apache/kafka/server/BootstrapControllersIntegrationTest.java:
##########
@@ -225,31 +228,93 @@ public void testIncrementalAlterConfigs(ClusterInstance 
clusterInstance) throws
         testIncrementalAlterConfigs(clusterInstance, false);
     }
 
+    @SuppressWarnings("unchecked")
     private void testIncrementalAlterConfigs(ClusterInstance clusterInstance, 
boolean usingBootstrapControllers) throws Exception {
+        Collection<Integer> nodeIds = usingBootstrapControllers ?
+                clusterInstance.controllerIds() : 
clusterInstance.brokers().keySet();
         try (Admin admin = Admin.create(adminConfig(clusterInstance, 
usingBootstrapControllers))) {
-            int nodeId = usingBootstrapControllers ?
-                    
clusterInstance.controllers().values().iterator().next().config().nodeId() :
-                    
clusterInstance.brokers().values().iterator().next().config().nodeId();
-            ConfigResource nodeResource = new ConfigResource(BROKER, "" + 
nodeId);
-            ConfigResource defaultResource = new ConfigResource(BROKER, "");
-            Map<ConfigResource, Collection<AlterConfigOp>> alterations = 
Map.of(
-                    nodeResource, List.of(new AlterConfigOp(new 
ConfigEntry("my.custom.config", "foo"), AlterConfigOp.OpType.SET)),
-                    defaultResource, List.of(new AlterConfigOp(new 
ConfigEntry("my.custom.config", "bar"), AlterConfigOp.OpType.SET))
-            );
-            admin.incrementalAlterConfigs(alterations).all().get(1, 
TimeUnit.MINUTES);
-            TestUtils.retryOnExceptionWithTimeout(30_000, () -> {
-                Config config = admin.describeConfigs(List.of(nodeResource)).
-                        all().get(1, TimeUnit.MINUTES).get(nodeResource);
-                ConfigEntry entry = config.entries().stream().
-                        filter(e -> e.name().equals("my.custom.config")).
-                        findFirst().orElseThrow();
-                assertEquals(DYNAMIC_BROKER_CONFIG, entry.source(),
-                        "Expected entry for my.custom.config to come from 
DYNAMIC_BROKER_CONFIG. " +
-                                "Instead, the entry was: " + entry);
-            });
+            for (int nodeId : nodeIds) {
+                ConfigResource nodeResource = new ConfigResource(BROKER, "" + 
nodeId);
+                ConfigResource defaultResource = new ConfigResource(BROKER, 
"");
+                String nodeMaxConnectionsValue = String.valueOf(1000 + nodeId);
+                String defaultMaxConnectionsValue = String.valueOf(2000 + 
nodeId);
+                String defaultConnectionRateValue = String.valueOf(2000 + 
nodeId);
+
+                // Set configs: MAX_CONNECTIONS_CONFIG for per-broker, both 
configs for default
+                Map<ConfigResource, Collection<AlterConfigOp>> alterations = 
Map.of(
+                        nodeResource, List.of(
+                                new AlterConfigOp(new 
ConfigEntry(SocketServerConfigs.MAX_CONNECTIONS_CONFIG, 
nodeMaxConnectionsValue), AlterConfigOp.OpType.SET)
+                        ),
+                        defaultResource, List.of(
+                                new AlterConfigOp(new 
ConfigEntry(SocketServerConfigs.MAX_CONNECTIONS_CONFIG, 
defaultMaxConnectionsValue), AlterConfigOp.OpType.SET),
+                                new AlterConfigOp(new 
ConfigEntry(SocketServerConfigs.MAX_CONNECTION_CREATION_RATE_CONFIG, 
defaultConnectionRateValue), AlterConfigOp.OpType.SET)
+                        )
+                );
+                admin.incrementalAlterConfigs(alterations).all().get(1, 
TimeUnit.MINUTES);
+                
+                // Verify per-broker configs: MAX_CONNECTIONS_CONFIG and 
MAX_CONNECTION_CREATION_RATE_CONFIG
+                verifyConfigValue(admin, nodeResource, 
SocketServerConfigs.MAX_CONNECTIONS_CONFIG,
+                        DYNAMIC_BROKER_CONFIG, nodeMaxConnectionsValue);
+                verifyConfigValue(admin, nodeResource, 
SocketServerConfigs.MAX_CONNECTION_CREATION_RATE_CONFIG,
+                        DYNAMIC_DEFAULT_BROKER_CONFIG, 
defaultConnectionRateValue);
+                
+                // Verify default broker configs: MAX_CONNECTIONS_CONFIG and 
MAX_CONNECTION_CREATION_RATE_CONFIG
+                verifyConfigValue(admin, defaultResource, 
SocketServerConfigs.MAX_CONNECTIONS_CONFIG,
+                        DYNAMIC_DEFAULT_BROKER_CONFIG, 
defaultMaxConnectionsValue);
+                verifyConfigValue(admin, defaultResource, 
SocketServerConfigs.MAX_CONNECTION_CREATION_RATE_CONFIG,
+                        DYNAMIC_DEFAULT_BROKER_CONFIG, 
defaultConnectionRateValue);
+
+                Object server = (usingBootstrapControllers ? 
clusterInstance.controllers() : clusterInstance.brokers()).get(nodeId);
+                // Verify that SocketServer has actually updated the max 
connection limit in ConnectionQuotas
+                // The per-broker config should take precedence over the 
default config
+                verifySocketServerMaxConnectionsUpdated(server, 
Integer.parseInt(nodeMaxConnectionsValue));
+                // Verify MAX_CONNECTION_CREATION_RATE_CONFIG is also updated
+                // The default config value should be used (per-broker config 
doesn't set this)
+                verifySocketServerMaxConnectionCreationRateUpdated(server, 
Integer.parseInt(defaultConnectionRateValue));
+            }
         }
     }
 
+    private void verifySocketServerMaxConnectionsUpdated(Object broker, int 
expectedMaxConnections) throws Exception {

Review Comment:
   `broker` -> `node`



##########
server/src/test/java/org/apache/kafka/server/BootstrapControllersIntegrationTest.java:
##########
@@ -225,31 +228,93 @@ public void testIncrementalAlterConfigs(ClusterInstance 
clusterInstance) throws
         testIncrementalAlterConfigs(clusterInstance, false);
     }
 
+    @SuppressWarnings("unchecked")

Review Comment:
   it is redundant



-- 
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]

Reply via email to