KAFKA-18837: Validate controller.quorum.fetch.timeout.ms is a positive value

raft/src/main/java/org/apache/kafka/raft/QuorumConfig.java

@@ -35,6 +35,7 @@
 import static org.apache.kafka.common.config.ConfigDef.Importance.HIGH;
 import static org.apache.kafka.common.config.ConfigDef.Importance.LOW;
 import static org.apache.kafka.common.config.ConfigDef.Importance.MEDIUM;
+import static org.apache.kafka.common.config.ConfigDef.Range.atLeast;
 import static org.apache.kafka.common.config.ConfigDef.Type.INT;
 import static org.apache.kafka.common.config.ConfigDef.Type.LIST;
 
@@ -69,23 +70,24 @@ public class QuorumConfig {
 
     public static final String QUORUM_ELECTION_TIMEOUT_MS_CONFIG = QUORUM_PREFIX + "election.timeout.ms";
     public static final String QUORUM_ELECTION_TIMEOUT_MS_DOC = "Maximum time in milliseconds to wait " +
-        "without being able to fetch from the leader before triggering a new election";
+        "without being able to fetch from the leader before triggering a new election, this value should large 0.";
     public static final int DEFAULT_QUORUM_ELECTION_TIMEOUT_MS = 1_000;
 
     public static final String QUORUM_FETCH_TIMEOUT_MS_CONFIG = QUORUM_PREFIX + "fetch.timeout.ms";
     public static final String QUORUM_FETCH_TIMEOUT_MS_DOC = "Maximum time without a successful fetch from " +
         "the current leader before becoming a candidate and triggering an election for voters; Maximum time " +
-        "a leader can go without receiving valid fetch or fetchSnapshot request from a majority of the quorum before resigning.";
+        "a leader can go without receiving valid fetch or fetchSnapshot request from a majority of the quorum before resigning, " +
+        "this value should large 0.";
     public static final int DEFAULT_QUORUM_FETCH_TIMEOUT_MS = 2_000;
 
     public static final String QUORUM_ELECTION_BACKOFF_MAX_MS_CONFIG = QUORUM_PREFIX + "election.backoff.max.ms";
     public static final String QUORUM_ELECTION_BACKOFF_MAX_MS_DOC = "Maximum time in milliseconds before starting new elections. " +
-        "This is used in the binary exponential backoff mechanism that helps prevent gridlocked elections";
+        "This is used in the binary exponential backoff mechanism that helps prevent gridlocked elections, this value should large 0.";
     public static final int DEFAULT_QUORUM_ELECTION_BACKOFF_MAX_MS = 1_000;
 
     public static final String QUORUM_LINGER_MS_CONFIG = QUORUM_PREFIX + "append.linger.ms";
     public static final String QUORUM_LINGER_MS_DOC = "The duration in milliseconds that the leader will " +
-        "wait for writes to accumulate before flushing them to disk.";
+        "wait for writes to accumulate before flushing them to disk, this value should large 0.";
 
     public static final int DEFAULT_QUORUM_LINGER_MS = 25;
 
@@ -102,12 +104,12 @@ public class QuorumConfig {
     public static final ConfigDef CONFIG_DEF =  new ConfigDef()
             .define(QUORUM_VOTERS_CONFIG, LIST, DEFAULT_QUORUM_VOTERS, new ControllerQuorumVotersValidator(), HIGH, QUORUM_VOTERS_DOC)
             .define(QUORUM_BOOTSTRAP_SERVERS_CONFIG, LIST, DEFAULT_QUORUM_BOOTSTRAP_SERVERS, new ControllerQuorumBootstrapServersValidator(), HIGH, QUORUM_BOOTSTRAP_SERVERS_DOC)
-            .define(QUORUM_ELECTION_TIMEOUT_MS_CONFIG, INT, DEFAULT_QUORUM_ELECTION_TIMEOUT_MS, null, HIGH, QUORUM_ELECTION_TIMEOUT_MS_DOC)
-            .define(QUORUM_FETCH_TIMEOUT_MS_CONFIG, INT, DEFAULT_QUORUM_FETCH_TIMEOUT_MS, null, HIGH, QUORUM_FETCH_TIMEOUT_MS_DOC)
-            .define(QUORUM_ELECTION_BACKOFF_MAX_MS_CONFIG, INT, DEFAULT_QUORUM_ELECTION_BACKOFF_MAX_MS, null, HIGH, QUORUM_ELECTION_BACKOFF_MAX_MS_DOC)
-            .define(QUORUM_LINGER_MS_CONFIG, INT, DEFAULT_QUORUM_LINGER_MS, null, MEDIUM, QUORUM_LINGER_MS_DOC)
-            .define(QUORUM_REQUEST_TIMEOUT_MS_CONFIG, INT, DEFAULT_QUORUM_REQUEST_TIMEOUT_MS, null, MEDIUM, QUORUM_REQUEST_TIMEOUT_MS_DOC)
-            .define(QUORUM_RETRY_BACKOFF_MS_CONFIG, INT, DEFAULT_QUORUM_RETRY_BACKOFF_MS, null, LOW, QUORUM_RETRY_BACKOFF_MS_DOC);
+            .define(QUORUM_ELECTION_TIMEOUT_MS_CONFIG, INT, DEFAULT_QUORUM_ELECTION_TIMEOUT_MS, atLeast(1), HIGH, QUORUM_ELECTION_TIMEOUT_MS_DOC)
+            .define(QUORUM_FETCH_TIMEOUT_MS_CONFIG, INT, DEFAULT_QUORUM_FETCH_TIMEOUT_MS, atLeast(1), HIGH, QUORUM_FETCH_TIMEOUT_MS_DOC)
+            .define(QUORUM_ELECTION_BACKOFF_MAX_MS_CONFIG, INT, DEFAULT_QUORUM_ELECTION_BACKOFF_MAX_MS, atLeast(1), HIGH, QUORUM_ELECTION_BACKOFF_MAX_MS_DOC)
+            .define(QUORUM_LINGER_MS_CONFIG, INT, DEFAULT_QUORUM_LINGER_MS, atLeast(1), MEDIUM, QUORUM_LINGER_MS_DOC)
+            .define(QUORUM_REQUEST_TIMEOUT_MS_CONFIG, INT, DEFAULT_QUORUM_REQUEST_TIMEOUT_MS, atLeast(1), MEDIUM, QUORUM_REQUEST_TIMEOUT_MS_DOC)
+            .define(QUORUM_RETRY_BACKOFF_MS_CONFIG, INT, DEFAULT_QUORUM_RETRY_BACKOFF_MS, atLeast(1), LOW, QUORUM_RETRY_BACKOFF_MS_DOC);
 
     private final List<String> voters;
     private final List<String> bootstrapServers;

raft/src/test/java/org/apache/kafka/raft/QuorumConfigTest.java

@@ -0,0 +1,66 @@
+/*
+ * 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.kafka.raft;
+
+import org.apache.kafka.common.config.AbstractConfig;
+import org.apache.kafka.common.config.ConfigException;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class QuorumConfigTest {
+    @Test
+    public void testPositiveConfig() {
+        verifyPositiveConfig(Map.of(QuorumConfig.QUORUM_ELECTION_TIMEOUT_MS_CONFIG, "0"));
+        verifyPositiveConfig(Map.of(QuorumConfig.QUORUM_FETCH_TIMEOUT_MS_CONFIG, "0"));
+        verifyPositiveConfig(Map.of(QuorumConfig.QUORUM_ELECTION_BACKOFF_MAX_MS_CONFIG, "0"));
+        verifyPositiveConfig(Map.of(QuorumConfig.QUORUM_LINGER_MS_CONFIG, "0"));
+        verifyPositiveConfig(Map.of(QuorumConfig.QUORUM_REQUEST_TIMEOUT_MS_CONFIG, "0"));
+        verifyPositiveConfig(Map.of(QuorumConfig.QUORUM_RETRY_BACKOFF_MS_CONFIG, "0"));
+    }
+
+    private void verifyPositiveConfig(Map<String, Object> overrideConfig) {
+        Map<String, Object> props = new HashMap<>();
+        props.put(QuorumConfig.QUORUM_VOTERS_CONFIG, "1@localhost:9092");
+        props.put(QuorumConfig.QUORUM_BOOTSTRAP_SERVERS_CONFIG, "localhost:9092");
+        props.put(QuorumConfig.QUORUM_ELECTION_TIMEOUT_MS_CONFIG, "10");
+        props.put(QuorumConfig.QUORUM_FETCH_TIMEOUT_MS_CONFIG, "10");
+        props.put(QuorumConfig.QUORUM_ELECTION_BACKOFF_MAX_MS_CONFIG, "10");
+        props.put(QuorumConfig.QUORUM_LINGER_MS_CONFIG, "10");
+        props.put(QuorumConfig.QUORUM_REQUEST_TIMEOUT_MS_CONFIG, "10");
+        props.put(QuorumConfig.QUORUM_RETRY_BACKOFF_MS_CONFIG, "10");
+
+        props.putAll(overrideConfig);
+
+        assertThrows(ConfigException.class, () -> new QuorumConfig(new QuorumTestConfig(props)));
+    }
+
+    private static class QuorumTestConfig extends AbstractConfig {
+        private final QuorumConfig quorumConfig;
+
+        public QuorumTestConfig(Map<?, ?> originals) {
+            super(QuorumConfig.CONFIG_DEF, originals, true);
+            quorumConfig = new QuorumConfig(this);
+        }
+
+        public QuorumConfig quorumConfig() {
+            return quorumConfig;
+        }
+    }
+}