[GitHub] [kafka] jsancio commented on a change in pull request #11179: KAFKA-13165: Validate node id, process role and quorum voters

[GitBox]

jsancio commented on a change in pull request #11179:
URL: https://github.com/apache/kafka/pull/11179#discussion_r683110083



##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -1949,6 +1951,15 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: 
Boolean, dynamicConfigO
       )
     }
 
+    val voterIds: Set[Integer] = if (usesSelfManagedQuorum) 
RaftConfig.parseVoterConnections(quorumVoters).asScala.keySet.toSet else 
Set.empty
+    if (processRoles.contains(ControllerRole)) {
+      // Ensure that controllers use their node.id as a voter in 
controller.quorum.voters 
+      require(voterIds.contains(nodeId), s"The controller must contain a voter 
for it's ${KafkaConfig.NodeIdProp}=$nodeId in 
${RaftConfig.QUORUM_VOTERS_CONFIG}.")
+    } else {
+      // Ensure that the broker's node.id is not an id in 
controller.quorum.voters
+      require(!voterIds.contains(nodeId), s"Since 
${KafkaConfig.ProcessRolesProp}=broker, the the broker's 
${KafkaConfig.NodeIdProp}=nodeId should not be in 
${RaftConfig.QUORUM_VOTERS_CONFIG}")

Review comment:
       How about "If ${....process role config...} does not contain the 
'controller' role, the node id $nodeId must not be included in the set of 
voters ${QUORUM_VOTERS_CONFIG}=$votersIds"?

##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -1949,6 +1951,15 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: 
Boolean, dynamicConfigO
       )
     }
 
+    val voterIds: Set[Integer] = if (usesSelfManagedQuorum) 
RaftConfig.parseVoterConnections(quorumVoters).asScala.keySet.toSet else 
Set.empty
+    if (processRoles.contains(ControllerRole)) {
+      // Ensure that controllers use their node.id as a voter in 
controller.quorum.voters 
+      require(voterIds.contains(nodeId), s"The controller must contain a voter 
for it's ${KafkaConfig.NodeIdProp}=$nodeId in 
${RaftConfig.QUORUM_VOTERS_CONFIG}.")

Review comment:
       How about "If ${....process role config...} contains the 'controller' 
role, the node id $nodeId must be included in the set of voters 
${QUORUM_VOTERS_CONFIG}=$votersIds"? 




-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org