Yunyung commented on code in PR #20319:
URL: https://github.com/apache/kafka/pull/20319#discussion_r2262947299
##########
tools/src/main/java/org/apache/kafka/tools/reassign/ReassignPartitionsCommand.java:
##########
@@ -922,14 +924,17 @@ private static void verifyBrokerIds(Admin adminClient,
Set<Integer> brokers) thr
* @return The string to print. We will only
print information about
* partitions that appear in the
proposed partition assignment.
*/
- static String
currentPartitionReplicaAssignmentToString(Map<TopicPartition, List<Integer>>
proposedParts,
+ static String currentPartitionReplicaAssignmentToString(Admin adminClient,
Review Comment:
Nit: Please also update the param in Java docs.
##########
tools/src/test/java/org/apache/kafka/tools/reassign/ReassignPartitionsUnitTest.java:
##########
@@ -436,29 +437,50 @@ public void testGenerateAssignmentWithFewerBrokers()
throws Exception {
@Test
public void testCurrentPartitionReplicaAssignmentToString() throws
Exception {
- Map<TopicPartition, List<Integer>> proposedParts = new HashMap<>();
-
- proposedParts.put(new TopicPartition("foo", 1), List.of(1, 2, 3));
- proposedParts.put(new TopicPartition("bar", 0), List.of(7, 8, 9));
-
- Map<TopicPartition, List<Integer>> currentParts = new HashMap<>();
-
- currentParts.put(new TopicPartition("foo", 0), List.of(1, 2, 3));
- currentParts.put(new TopicPartition("foo", 1), List.of(4, 5, 6));
- currentParts.put(new TopicPartition("bar", 0), List.of(7, 8));
- currentParts.put(new TopicPartition("baz", 0), List.of(10, 11, 12));
+ try (MockAdminClient adminClient = new MockAdminClient.Builder()
+ .numBrokers(6)
+ .brokerLogDirs(List.of(
+ List.of("/tmp/broker0/logs"),
+ List.of("/tmp/broker1/logs"),
+ List.of("/tmp/broker2/logs"),
+ List.of("/tmp/broker3/logs"),
+ List.of("/tmp/broker4/logs"),
+ List.of("/tmp/broker5/logs")
+ ))
+ .build()
+ ) {
+
+ List<Node> brokers = adminClient.brokers();
+ adminClient.addTopic(false, "foo", List.of(
+ new TopicPartitionInfo(1, brokers.get(1),
+ List.of(brokers.get(1), brokers.get(2), brokers.get(3)),
+ List.of(brokers.get(1), brokers.get(2), brokers.get(3)))
+ ), Map.of());
+
+ adminClient.addTopic(false, "bar", List.of(
+ new TopicPartitionInfo(0, brokers.get(4),
+ List.of(brokers.get(4), brokers.get(5)),
+ List.of(brokers.get(4), brokers.get(5)))
+ ), Map.of());
+
+ Map<TopicPartition, List<Integer>> proposedParts = new HashMap<>();
+ proposedParts.put(new TopicPartition("foo", 1), List.of(0, 1, 2));
+ proposedParts.put(new TopicPartition("bar", 0), List.of(3, 4, 5));
+
+ Map<TopicPartition, List<Integer>> currentParts = new HashMap<>();
+ currentParts.put(new TopicPartition("foo", 1), List.of(1, 2, 3));
+ currentParts.put(new TopicPartition("bar", 0), List.of(4, 5));
- assertEquals(String.join(System.lineSeparator(),
- "Current partition replica assignment",
- "",
- "{\"version\":1,\"partitions\":" +
-
"[{\"topic\":\"bar\",\"partition\":0,\"replicas\":[7,8],\"log_dirs\":[\"any\",\"any\"]},"
+
-
"{\"topic\":\"foo\",\"partition\":1,\"replicas\":[4,5,6],\"log_dirs\":[\"any\",\"any\",\"any\"]}]"
+
- "}",
- "",
- "Save this to use as the --reassignment-json-file option during
rollback"),
- currentPartitionReplicaAssignmentToString(proposedParts,
currentParts)
- );
+ assertEquals(String.join(System.lineSeparator(),
+ "Current partition replica assignment",
+ "",
+
"{\"version\":1,\"partitions\":[{\"topic\":\"bar\",\"partition\":0,\"replicas\":[4,5],\"log_dirs\":[\"/tmp/broker4/logs\",\"/tmp/broker4/logs\"]},"
+
+
"{\"topic\":\"foo\",\"partition\":1,\"replicas\":[1,2,3],\"log_dirs\":[\"any\",\"any\",\"any\"]}]}",
Review Comment:
'any' looks like a bug. I believe this is caused by this line:
https://github.com/apache/kafka/blob/8deb6c6911616f887ebb2678f3f12ee1da09a618/clients/src/test/java/org/apache/kafka/clients/admin/MockAdminClient.java#L1135
Is bug i think
--
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
