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/03/11 18:18:24 UTC

[GitHub] [kafka] cmccabe opened a new pull request #10306: MINOR: socket setup should be 30s for AdminCLient

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


   socket.connection.setup.timeout.max.ms should be 30 seconds for
   AdminClient.  The current value of 127 seconds is longer than the
   default API timeout.  With that timeout, AdminClient API calls will fail
   before the connection setup timeout ever hits.  This defeats the purpose
   of connection setup timeout.
   
   A value of 30 seconds is a good compromise since it gives the API call
   another 30 seconds to find a new node and make the call, before the
   timeout hits.


----------------------------------------------------------------
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] cmccabe merged pull request #10306: MINOR: socket setup max should be 30 seconds

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


   


----------------------------------------------------------------
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] hachikuji commented on a change in pull request #10306: MINOR: socket setup should be 30s for AdminCLient

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



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java
##########
@@ -162,7 +162,7 @@
                                         CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_DOC)
                                 .define(SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG,
                                         Type.LONG,
-                                        CommonClientConfigs.DEFAULT_SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS,

Review comment:
       I think the producer and consumer have the same problem. Maybe we can just change the constant?




----------------------------------------------------------------
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] ijuma commented on pull request #10306: MINOR: socket setup max should be 30 seconds

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


   This needs a JIRA and a release note, no?


----------------------------------------------------------------
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] cmccabe commented on pull request #10306: MINOR: socket setup max should be 30 seconds

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


   Committed to trunk and 2.8


----------------------------------------------------------------
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] ijuma commented on pull request #10306: MINOR: socket setup max should be 30 seconds

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


   @cmccabe We should update the KIP and share a note in the mailing list thread. Also, was this released with `127` as the default or did we change it before the first release? If the former, we should add a release note, 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.

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



[GitHub] [kafka] cmccabe commented on a change in pull request #10306: MINOR: socket setup max should be 30 seconds

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



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java
##########
@@ -162,7 +162,7 @@
                                         CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_DOC)
                                 .define(SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_CONFIG,
                                         Type.LONG,
-                                        CommonClientConfigs.DEFAULT_SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS,

Review comment:
       seems reasonable.




----------------------------------------------------------------
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] hachikuji commented on pull request #10306: MINOR: socket setup max should be 30 seconds

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


   cc @vvcephei This might be a good one for 2.8 since it is a simple fix. 


----------------------------------------------------------------
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] cmccabe commented on pull request #10306: MINOR: socket setup max should be 30 seconds

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


   I retried the tests that failed locally and they all worked for me.  Will commit in a minute.


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