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/06/04 01:32:01 UTC

[GitHub] [kafka] socutes opened a new pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

socutes opened a new pull request #10815:
URL: https://github.com/apache/kafka/pull/10815


   https://issues.apache.org/jira/browse/KAFKA-9220 mentions kafka-preferred-replica-election.sh script hard-coded timeout problems. I see a similar problem with kafka-leader-election.sh.
   
   I would like to add a --timeout parameter to kafka-leader-election.sh to control the request timeout. To solve similar problems.


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

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



[GitHub] [kafka] socutes commented on pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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


   Get! Thank you very much.


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

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



[GitHub] [kafka] showuon commented on pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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


   Yes, KIP is necessary for this change. ref: https://cwiki.apache.org/confluence/display/kafka/kafka+improvement+proposals


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

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



[GitHub] [kafka] socutes commented on a change in pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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



##########
File path: core/src/main/scala/kafka/admin/LeaderElectionCommand.scala
##########
@@ -283,6 +284,14 @@ private final class LeaderElectionCommandOptions(args: Array[String]) extends Co
     .describedAs("election type")
     .withValuesConvertedBy(ElectionTypeConverter)
 
+  val requestTimeout = parser
+    .accepts(
+      "timeout",
+      CommonClientConfigs.REQUEST_TIMEOUT_MS_DOC)

Review comment:
       Thank you very much! AdminClientConfig REQUEST_TIMEOUT_MS_DOC is a private property. I will rewrite the instructions




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

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



[GitHub] [kafka] socutes commented on pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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


   @showuon , please help review this PR. Thanks!


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

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



[GitHub] [kafka] wenbingshen commented on a change in pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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



##########
File path: core/src/main/scala/kafka/admin/LeaderElectionCommand.scala
##########
@@ -283,6 +284,14 @@ private final class LeaderElectionCommandOptions(args: Array[String]) extends Co
     .describedAs("election type")
     .withValuesConvertedBy(ElectionTypeConverter)
 
+  val requestTimeout = parser
+    .accepts(
+      "timeout",
+      CommonClientConfigs.REQUEST_TIMEOUT_MS_DOC)

Review comment:
       This configuration is available in AdminClientConfig `AdminClientConfig.REQUEST_TIMEOUT_MS_DOC`. Can we not introduce CommonClientConfigs as we prefer clean code?

##########
File path: core/src/main/scala/kafka/admin/LeaderElectionCommand.scala
##########
@@ -26,6 +26,7 @@ import kafka.utils.CoreUtils
 import kafka.utils.Implicits._
 import kafka.utils.Json
 import kafka.utils.Logging
+import org.apache.kafka.clients.CommonClientConfigs

Review comment:
       AdminClientConfig has been introduced.




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

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



[GitHub] [kafka] socutes commented on pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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


   @showuon , please help review this PR. Thanks!


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

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



[GitHub] [kafka] wenbingshen commented on pull request #10815: KAFKA-12885: Add the --timeout property to kafka-leader-election.sh

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


   @socutes Thanks for your reply. We need a KIP for adding a new flag to a command line tool as it is considered as part of the public API.


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

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