[GitHub] [kafka] akhileshchg commented on a diff in pull request #12106: KAFKA-13861: Fix the validateOnly behavior for CreatePartitions requests in KRaft mode

[GitBox <gi...@apache.org>]

akhileshchg commented on code in PR #12106:
URL: https://github.com/apache/kafka/pull/12106#discussion_r863095080


##########
core/src/test/java/kafka/test/MockController.java:
##########
@@ -410,10 +410,16 @@ public CompletableFuture<UpdateFeaturesResponseData> updateFeatures(
         throw new UnsupportedOperationException();
     }
 
+    boolean lastCreatePartitionsValidateOnly = false;

Review Comment:
   The `MockController` doesn't maintain any state of the topics/partitions that are created and deleted. So, I thought this was a more straightforward fix to check if the `validateOnly` flag is used. Please let met know if you have some other ideas.



##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -1364,7 +1364,25 @@ Boolean isBrokerUnfenced(int brokerId) {
                 setErrorCode(apiError.error().code()).
                 setErrorMessage(apiError.message()));
         }
-        return new ControllerResult<>(records, results, true);
+        StringBuilder resultsBuilder = new StringBuilder();

Review Comment:
   Sure. That seems reasonable. I did this to match the `createTopics` log statement.



##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -767,7 +767,7 @@ class ControllerApis(val requestChannel: RequestChannel,
           setErrorCode(TOPIC_AUTHORIZATION_FAILED.code))
       }
     }
-    controller.createPartitions(context, topics).thenApply { results =>
+    controller.createPartitions(context, topics, request.validateOnly()).thenApply { results =>

Review Comment:
   Done.



##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -767,7 +767,7 @@ class ControllerApis(val requestChannel: RequestChannel,
           setErrorCode(TOPIC_AUTHORIZATION_FAILED.code))
       }
     }
-    controller.createPartitions(context, topics).thenApply { results =>
+    controller.createPartitions(context, topics, request.validateOnly()).thenApply { results =>

Review Comment:
   I enabled the test for both KRaft and Zk modes now. The existing test is robust and covers all the cases.



##########
metadata/src/main/java/org/apache/kafka/controller/QuorumController.java:
##########
@@ -1664,13 +1664,14 @@ public CompletableFuture<UpdateFeaturesResponseData> updateFeatures(
     @Override
     public CompletableFuture<List<CreatePartitionsTopicResult>> createPartitions(

Review Comment:
   That's a good idea. Will do this.



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