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/19 14:42:46 UTC
[GitHub] [kafka] ijuma opened a new pull request #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
ijuma opened a new pull request #10362:
URL: https://github.com/apache/kafka/pull/10362
KIP-500 is not particularly descriptive.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-803019738
@cmccabe @hachikuji I considered removing the prefix from the file, but I thought people may copy the files to other locations and the file name should be descriptive. If you both still feel it should be renamed, I am happy to do it though. Let me know.
--
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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-803193725
Merged to trunk and cherry-picked to 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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-803188610
Unrelated flaky failure:
> RebalanceSourceConnectorsIntegrationTest.testMultipleWorkersRejoining
--
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 merged pull request #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #10362:
URL: https://github.com/apache/kafka/pull/10362
--
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] rondagostino commented on a change in pull request #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#discussion_r597775208
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -663,19 +663,18 @@ object KafkaConfig {
val ConnectionSetupTimeoutMsDoc = CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_DOC
val ConnectionSetupTimeoutMaxMsDoc = CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_DOC
- /** KIP-500 Config Documentation */
+ /** Self-managed mode configs */
val ProcessRolesDoc = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. " +
- "This configuration is only for clusters upgraded for KIP-500, which replaces the dependence on Zookeeper with " +
- "a self-managed Raft quorum. Leave this config undefined or empty for Zookeeper clusters."
+ "This configuration is only for clusters in self-managed mode, which rely on a Raft quorum instead of ZooKeeper. Leave this config undefined or empty for Zookeeper clusters."
val InitialBrokerRegistrationTimeoutMsDoc = "When initially registering with the controller quorum, the number of milliseconds to wait before declaring failure and exiting the broker process."
- val BrokerHeartbeatIntervalMsDoc = "The length of time in milliseconds between broker heartbeats. Used when running in KIP-500 mode."
- val BrokerSessionTimeoutMsDoc = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in KIP-500 mode."
+ val BrokerHeartbeatIntervalMsDoc = "The length of time in milliseconds between broker heartbeats. Used when running in self-managed mode."
+ val BrokerSessionTimeoutMsDoc = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in self-managed mode."
val NodeIdDoc = "The node ID associated with the roles this process is playing when `process.roles` is non-empty. " +
"This is required configuration when the self-managed quorum is enabled."
- val MetadataLogDirDoc = "This configuration determines where we put the metadata log for clusters upgraded to " +
- "KIP-500. If it is not set, the metadata log is placed in the first log directory from log.dirs."
- val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the KIP-500 controller. This is required " +
- "if this process is a KIP-500 controller. The ZK-based controller will not use this configuration."
+ val MetadataLogDirDoc = "This configuration determines where we put the metadata log for clusters in self-managed mode. " +
+ "If it is not set, the metadata log is placed in the first log directory from log.dirs."
+ val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the self-managed controller. This is required " +
+ "if this process is a self-managed controller. The ZK-based controller will not use this configuration."
Review comment:
> This is required if this process is a self-managed controller.
This is actually required for self-managed brokers as well -- it is how the broker figures out what security protocol to use when connecting to the controller.quorum.voters.
Maybe `s/if this process is a self-managed controller/if this process is a self-managed broker and/or controller/`
--
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 a change in pull request #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#discussion_r597885573
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -663,19 +663,18 @@ object KafkaConfig {
val ConnectionSetupTimeoutMsDoc = CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_DOC
val ConnectionSetupTimeoutMaxMsDoc = CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_DOC
- /** KIP-500 Config Documentation */
+ /** Self-managed mode configs */
val ProcessRolesDoc = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. " +
- "This configuration is only for clusters upgraded for KIP-500, which replaces the dependence on Zookeeper with " +
- "a self-managed Raft quorum. Leave this config undefined or empty for Zookeeper clusters."
+ "This configuration is only for clusters in self-managed mode, which rely on a Raft quorum instead of ZooKeeper. Leave this config undefined or empty for Zookeeper clusters."
val InitialBrokerRegistrationTimeoutMsDoc = "When initially registering with the controller quorum, the number of milliseconds to wait before declaring failure and exiting the broker process."
- val BrokerHeartbeatIntervalMsDoc = "The length of time in milliseconds between broker heartbeats. Used when running in KIP-500 mode."
- val BrokerSessionTimeoutMsDoc = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in KIP-500 mode."
+ val BrokerHeartbeatIntervalMsDoc = "The length of time in milliseconds between broker heartbeats. Used when running in self-managed mode."
+ val BrokerSessionTimeoutMsDoc = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in self-managed mode."
val NodeIdDoc = "The node ID associated with the roles this process is playing when `process.roles` is non-empty. " +
"This is required configuration when the self-managed quorum is enabled."
- val MetadataLogDirDoc = "This configuration determines where we put the metadata log for clusters upgraded to " +
- "KIP-500. If it is not set, the metadata log is placed in the first log directory from log.dirs."
- val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the KIP-500 controller. This is required " +
- "if this process is a KIP-500 controller. The ZK-based controller will not use this configuration."
+ val MetadataLogDirDoc = "This configuration determines where we put the metadata log for clusters in self-managed mode. " +
+ "If it is not set, the metadata log is placed in the first log directory from log.dirs."
+ val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the self-managed controller. This is required " +
+ "if this process is a self-managed controller. The ZK-based controller will not use this configuration."
Review comment:
Applied Colin's 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
[GitHub] [kafka] cmccabe commented on a change in pull request #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
cmccabe commented on a change in pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#discussion_r597824680
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -663,19 +663,18 @@ object KafkaConfig {
val ConnectionSetupTimeoutMsDoc = CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MS_DOC
val ConnectionSetupTimeoutMaxMsDoc = CommonClientConfigs.SOCKET_CONNECTION_SETUP_TIMEOUT_MAX_MS_DOC
- /** KIP-500 Config Documentation */
+ /** Self-managed mode configs */
val ProcessRolesDoc = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. " +
- "This configuration is only for clusters upgraded for KIP-500, which replaces the dependence on Zookeeper with " +
- "a self-managed Raft quorum. Leave this config undefined or empty for Zookeeper clusters."
+ "This configuration is only for clusters in self-managed mode, which rely on a Raft quorum instead of ZooKeeper. Leave this config undefined or empty for Zookeeper clusters."
val InitialBrokerRegistrationTimeoutMsDoc = "When initially registering with the controller quorum, the number of milliseconds to wait before declaring failure and exiting the broker process."
- val BrokerHeartbeatIntervalMsDoc = "The length of time in milliseconds between broker heartbeats. Used when running in KIP-500 mode."
- val BrokerSessionTimeoutMsDoc = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in KIP-500 mode."
+ val BrokerHeartbeatIntervalMsDoc = "The length of time in milliseconds between broker heartbeats. Used when running in self-managed mode."
+ val BrokerSessionTimeoutMsDoc = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in self-managed mode."
val NodeIdDoc = "The node ID associated with the roles this process is playing when `process.roles` is non-empty. " +
"This is required configuration when the self-managed quorum is enabled."
- val MetadataLogDirDoc = "This configuration determines where we put the metadata log for clusters upgraded to " +
- "KIP-500. If it is not set, the metadata log is placed in the first log directory from log.dirs."
- val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the KIP-500 controller. This is required " +
- "if this process is a KIP-500 controller. The ZK-based controller will not use this configuration."
+ val MetadataLogDirDoc = "This configuration determines where we put the metadata log for clusters in self-managed mode. " +
+ "If it is not set, the metadata log is placed in the first log directory from log.dirs."
+ val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the self-managed controller. This is required " +
+ "if this process is a self-managed controller. The ZK-based controller will not use this configuration."
Review comment:
How about "this is required if this process is part of a self-managed cluster"
--
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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-803129459
I renamed the files, made a few tweaks to the readme and went through it to make sure it still works. Really cool btw, exciting to see all of this working. :)
--
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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-802966449
This all seems reasonable. It's nice to have a link from the main readme.
One nitpick: If we're going to have a subdirectory for the self-managed configurations, then we don't need to also have "self-managed" in the file name, I think. For example, `./config/self-managed/self-managed-combined.properties` can just be `./config/self-managed/combined.properties`, etc.
--
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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-803089875
I don't have a strong opinion but I think on balance I'd prefer to remove the prefix
--
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 #10362: MINOR: Use self-managed mode instead of KIP-500 and nozk
Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10362:
URL: https://github.com/apache/kafka/pull/10362#issuecomment-802992884
I agree with Colin that we should drop the "self-managed" prefix from the files under the directory with the same name.
--
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