You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/07/14 07:09:49 UTC

[GitHub] [kafka] cmccabe opened a new pull request #11048: KAFKA-13083: In KRaft mode, fix issue with ISR in manual partition assignments

cmccabe opened a new pull request #11048:
URL: https://github.com/apache/kafka/pull/11048


   When creating a new topic that includes manually assigned partitions, we
   must check if the nodes are unfenced before adding them to the new ISR.
   
   This PR adds two unit tests of this behavior in
   ReplicationControlManagerTest, and fixes a test in QuorumControllerTest
   that needed to unfenced nodes. Also some test edits to reduce verbosity
   and repetitiveness.


-- 
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-unsubscribe@kafka.apache.org

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



[GitHub] [kafka] cmccabe merged pull request #11048: KAFKA-13083: In KRaft mode, fix issue with ISR in manual partition assignments

Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #11048:
URL: https://github.com/apache/kafka/pull/11048


   


-- 
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-unsubscribe@kafka.apache.org

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



[GitHub] [kafka] cmccabe commented on pull request #11048: KAFKA-13083: In KRaft mode, fix issue with ISR in manual partition assignments

Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #11048:
URL: https://github.com/apache/kafka/pull/11048#issuecomment-880035001


   > I'm assuming we don't need a similar check for the automatic assignment path since the replica placer will only select unfenced replicas.
   
   Right.


-- 
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-unsubscribe@kafka.apache.org

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



[GitHub] [kafka] cmccabe commented on a change in pull request #11048: KAFKA-13083: In KRaft mode, fix issue with ISR in manual partition assignments

Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #11048:
URL: https://github.com/apache/kafka/pull/11048#discussion_r669790479



##########
File path: metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java
##########
@@ -377,8 +377,20 @@ private ApiError createTopic(CreatableTopic topic,
                 validateManualPartitionAssignment(assignment.brokerIds(), replicationFactor);
                 replicationFactor = OptionalInt.of(assignment.brokerIds().size());
                 int[] replicas = Replicas.toArray(assignment.brokerIds());
+                List<Integer> isr = new ArrayList<>();

Review comment:
       Yeah, we can use a stream.




-- 
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-unsubscribe@kafka.apache.org

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



[GitHub] [kafka] mumrah commented on a change in pull request #11048: KAFKA-13083: In KRaft mode, fix issue with ISR in manual partition assignments

Posted by GitBox <gi...@apache.org>.
mumrah commented on a change in pull request #11048:
URL: https://github.com/apache/kafka/pull/11048#discussion_r669693620



##########
File path: metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java
##########
@@ -377,8 +377,20 @@ private ApiError createTopic(CreatableTopic topic,
                 validateManualPartitionAssignment(assignment.brokerIds(), replicationFactor);
                 replicationFactor = OptionalInt.of(assignment.brokerIds().size());
                 int[] replicas = Replicas.toArray(assignment.brokerIds());
+                List<Integer> isr = new ArrayList<>();

Review comment:
       I think we can reduce the conversions between list and array here. We could also use a stream, e.g.,
   ```java
   assignment.brokerIds().stream().filter(clusterControl::unfenced)
   ```




-- 
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-unsubscribe@kafka.apache.org

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



[GitHub] [kafka] cmccabe commented on pull request #11048: KAFKA-13083: In KRaft mode, fix issue with ISR in manual partition assignments

Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #11048:
URL: https://github.com/apache/kafka/pull/11048#issuecomment-880053828


   I updated this and fixed a case in createPartitions where the same issue existed (added a test for that as well)


-- 
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-unsubscribe@kafka.apache.org

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