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/02/22 22:24:45 UTC
[GitHub] [kafka] cmccabe opened a new pull request #10179: MINOR: tune KIP-631 configurations
cmccabe opened a new pull request #10179:
URL: https://github.com/apache/kafka/pull/10179
Since we expect KIP-631 controller failovers to be fairly cheap, tune
the default raft configuration parameters so that we detect node
failures more quickly.
Reduce the broker session timeout as well so that broker failures are
detected more quickly.
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #10179:
URL: https://github.com/apache/kafka/pull/10179
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#discussion_r583247616
##########
File path: raft/src/main/java/org/apache/kafka/raft/RaftConfig.java
##########
@@ -79,12 +84,12 @@
public static final String QUORUM_REQUEST_TIMEOUT_MS_CONFIG = QUORUM_PREFIX +
CommonClientConfigs.REQUEST_TIMEOUT_MS_CONFIG;
public static final String QUORUM_REQUEST_TIMEOUT_MS_DOC = CommonClientConfigs.REQUEST_TIMEOUT_MS_DOC;
- public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 20_000;
+ public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 4_000;
Review comment:
Does it make sense for this to be lower than the fetch timeout? After the fetch timeout, we effectively give up on pending requests anyway.
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#issuecomment-783724833
Can we add a comment to the code explaining why these timeouts are lower than the ZK equivalent ones (similar to what's in the PR description).
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#issuecomment-783922535
I added some JavaDoc to the RaftConfig class
----------------------------------------------------------------
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 edited a comment on pull request #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
ijuma edited a comment on pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#issuecomment-783724833
Can we add a comment to the code explaining why these timeouts are lower than the ZK equivalent ones (similar to what's in the PR description). May even be worth including it in the documentation string (people will wonder).
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#issuecomment-786277818
Overall, I'm ok with the lower timeouts for 2.8. I do think it's highly likely we'll end up increasing them after we get some operational experience with the new code. It's just the nature of these things. Flaky networks are much more common than true node failures.
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#discussion_r583254723
##########
File path: raft/src/main/java/org/apache/kafka/raft/RaftConfig.java
##########
@@ -79,12 +84,12 @@
public static final String QUORUM_REQUEST_TIMEOUT_MS_CONFIG = QUORUM_PREFIX +
CommonClientConfigs.REQUEST_TIMEOUT_MS_CONFIG;
public static final String QUORUM_REQUEST_TIMEOUT_MS_DOC = CommonClientConfigs.REQUEST_TIMEOUT_MS_DOC;
- public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 20_000;
+ public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 4_000;
Review comment:
Let's set them both to 3 seconds
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#discussion_r583254723
##########
File path: raft/src/main/java/org/apache/kafka/raft/RaftConfig.java
##########
@@ -79,12 +84,12 @@
public static final String QUORUM_REQUEST_TIMEOUT_MS_CONFIG = QUORUM_PREFIX +
CommonClientConfigs.REQUEST_TIMEOUT_MS_CONFIG;
public static final String QUORUM_REQUEST_TIMEOUT_MS_DOC = CommonClientConfigs.REQUEST_TIMEOUT_MS_DOC;
- public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 20_000;
+ public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 4_000;
Review comment:
OK, let's set them both to 2 seconds
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#discussion_r583247616
##########
File path: raft/src/main/java/org/apache/kafka/raft/RaftConfig.java
##########
@@ -79,12 +84,12 @@
public static final String QUORUM_REQUEST_TIMEOUT_MS_CONFIG = QUORUM_PREFIX +
CommonClientConfigs.REQUEST_TIMEOUT_MS_CONFIG;
public static final String QUORUM_REQUEST_TIMEOUT_MS_DOC = CommonClientConfigs.REQUEST_TIMEOUT_MS_DOC;
- public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 20_000;
+ public static final int DEFAULT_QUORUM_REQUEST_TIMEOUT_MS = 4_000;
Review comment:
Does it make sense for this to be higher than the fetch timeout? After the fetch timeout, we effectively give up on pending requests anyway.
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#issuecomment-783923893
@hachikuji can you please review these timeouts?
----------------------------------------------------------------
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 #10179: MINOR: tune KIP-631 configurations
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10179:
URL: https://github.com/apache/kafka/pull/10179#issuecomment-786277886
> We have a hard-coded fetch max wait time here: https://github.com/apache/kafka/blob/trunk/raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java#L143. I think it would be a good idea to reduce this to 500ms to give a little more margin for response delays given the lower default timeout. (We should probably also consider making this a configuration, but we can do it separately)
good catch. will change
----------------------------------------------------------------
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