You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2024/12/29 13:01:17 UTC
[PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
m1a2st opened a new pull request, #18346:
URL: https://github.com/apache/kafka/pull/18346
These document in my local machine.
### 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18346:
URL: https://github.com/apache/kafka/pull/18346#issuecomment-2598245281
Thanks for @mimaison, @frankvicky review, addressed all comments
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison merged PR #18346:
URL: https://github.com/apache/kafka/pull/18346
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1922516081
##########
server/src/main/java/org/apache/kafka/network/SocketServerConfigs.java:
##########
@@ -77,8 +77,8 @@ public class SocketServerConfigs {
public static final String ADVERTISED_LISTENERS_CONFIG = "advertised.listeners";
public static final String ADVERTISED_LISTENERS_DOC = String.format("Specifies the listener addresses that the Kafka brokers will advertise to clients and other brokers." +
" The config is useful where the actual listener configuration <code>%s</code> does not represent the addresses that clients should" +
- " use to connect, such as in cloud environments. In environments using ZooKeeper, these addresses are published to ZooKeeper." +
- " In Kraft mode, the address would be published to and managed by kraft controller, the brokers would pull these data from controller as needed." +
+ " use to connect, such as in cloud environments." +
+ " The address would be published to and managed by the KRaft controller, the brokers pull these data from the controller as needed." +
Review Comment:
Oh sorry, this is my bad
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1917514065
##########
server/src/main/java/org/apache/kafka/server/config/KRaftConfigs.java:
##########
@@ -36,29 +36,27 @@
public class KRaftConfigs {
/** KRaft mode configs */
public static final String PROCESS_ROLES_CONFIG = "process.roles";
- public static final String PROCESS_ROLES_DOC = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. " +
- "This configuration is only applicable for clusters in KRaft (Kafka Raft) mode (instead of ZooKeeper). Leave this config undefined or empty for ZooKeeper clusters.";
-
+ public static final String PROCESS_ROLES_DOC = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. ";
+
public static final String INITIAL_BROKER_REGISTRATION_TIMEOUT_MS_CONFIG = "initial.broker.registration.timeout.ms";
public static final int INITIAL_BROKER_REGISTRATION_TIMEOUT_MS_DEFAULT = 60000;
public static final String INITIAL_BROKER_REGISTRATION_TIMEOUT_MS_DOC = "When initially registering with the controller quorum, the number of milliseconds to wait before declaring failure and exiting the broker process.";
public static final String BROKER_HEARTBEAT_INTERVAL_MS_CONFIG = "broker.heartbeat.interval.ms";
public static final int BROKER_HEARTBEAT_INTERVAL_MS_DEFAULT = 2000;
- public static final String BROKER_HEARTBEAT_INTERVAL_MS_DOC = "The length of time in milliseconds between broker heartbeats. Used when running in KRaft mode.";
+ public static final String BROKER_HEARTBEAT_INTERVAL_MS_DOC = "The length of time in milliseconds between broker heartbeats.";
public static final String BROKER_SESSION_TIMEOUT_MS_CONFIG = "broker.session.timeout.ms";
public static final int BROKER_SESSION_TIMEOUT_MS_DEFAULT = 9000;
- public static final String BROKER_SESSION_TIMEOUT_MS_DOC = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in KRaft mode.";
+ public static final String BROKER_SESSION_TIMEOUT_MS_DOC = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made.";
public static final String NODE_ID_CONFIG = "node.id";
public static final int EMPTY_NODE_ID = -1;
- public static final String NODE_ID_DOC = "The node ID associated with the roles this process is playing when <code>process.roles</code> is non-empty. " +
- "This is required configuration when running in KRaft mode.";
+ public static final String NODE_ID_DOC = "The node ID associated with the roles this process is playing when <code>process.roles</code> is non-empty.";
Review Comment:
We should keep the statement indicating this is a required configuration.
##########
server/src/main/java/org/apache/kafka/server/config/KRaftConfigs.java:
##########
@@ -79,8 +77,7 @@ public class KRaftConfigs {
public static final String CONTROLLER_LISTENER_NAMES_CONFIG = "controller.listener.names";
public static final String CONTROLLER_LISTENER_NAMES_DOC = "A comma-separated list of the names of the listeners used by the controller. This is required " +
- "if running in KRaft mode. When communicating with the controller quorum, the broker will always use the first listener in this list.\n " +
- "Note: The ZooKeeper-based controller should not set this configuration.";
+ "in KRaft mode. When communicating with the controller quorum, the broker will always use the first listener in this list.";
Review Comment:
If we only have kraft mode, why do we need to say "in kraft mode"?
This implies there are other modes and is very confusing.
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18346:
URL: https://github.com/apache/kafka/pull/18346#issuecomment-2590616937
Hello @dajac, if you have free cycle. Please take a look this PR, I think it should cherrypick to 4.0 branch, Thank you :)
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #18346:
URL: https://github.com/apache/kafka/pull/18346#issuecomment-2602664926
Thank @mimaison for your review and patience. Im so sorry for my poor english.
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1929621640
##########
server-common/src/main/java/org/apache/kafka/server/config/QuotaConfig.java:
##########
@@ -54,9 +54,8 @@ public class QuotaConfig {
public static final String CLIENT_QUOTA_CALLBACK_CLASS_CONFIG = "client.quota.callback.class";
public static final String CLIENT_QUOTA_CALLBACK_CLASS_DOC = "The fully qualified name of a class that implements the ClientQuotaCallback interface, " +
- "which is used to determine quota limits applied to client requests. By default, the <user> and <client-id> " +
- "quotas that are stored in ZooKeeper are applied. For any given request, the most specific quota that matches the user principal " +
Review Comment:
The explanation of the default behavior is still useful (without mentioning zk specifically).
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1922512385
##########
server/src/main/java/org/apache/kafka/network/SocketServerConfigs.java:
##########
@@ -77,8 +77,8 @@ public class SocketServerConfigs {
public static final String ADVERTISED_LISTENERS_CONFIG = "advertised.listeners";
public static final String ADVERTISED_LISTENERS_DOC = String.format("Specifies the listener addresses that the Kafka brokers will advertise to clients and other brokers." +
" The config is useful where the actual listener configuration <code>%s</code> does not represent the addresses that clients should" +
- " use to connect, such as in cloud environments. In environments using ZooKeeper, these addresses are published to ZooKeeper." +
- " In Kraft mode, the address would be published to and managed by kraft controller, the brokers would pull these data from controller as needed." +
+ " use to connect, such as in cloud environments." +
+ " The address would be published to and managed by the KRaft controller, the brokers pull these data from the controller as needed." +
Review Comment:
Actually I think we should use present tense. Something like `The addresses are published ...`
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1920064952
##########
server/src/main/java/org/apache/kafka/server/config/ServerConfigs.java:
##########
@@ -46,9 +46,7 @@ public class ServerConfigs {
public static final String BROKER_ID_CONFIG = "broker.id";
public static final int BROKER_ID_DEFAULT = -1;
- public static final String BROKER_ID_DOC = "The broker id for this server. If unset, a unique broker id will be generated." +
- "To avoid conflicts between ZooKeeper generated broker id's and user configured broker id's, generated broker ids " +
- "start from " + RESERVED_BROKER_MAX_ID_CONFIG + " + 1.";
+ public static final String BROKER_ID_DOC = "The broker id for this server. If unset, a unique broker id will be generated.";
Review Comment:
I don't think this is correct. In KRaft mode, broker ids are not generated.
##########
server/src/main/java/org/apache/kafka/network/SocketServerConfigs.java:
##########
@@ -77,7 +77,7 @@ public class SocketServerConfigs {
public static final String ADVERTISED_LISTENERS_CONFIG = "advertised.listeners";
public static final String ADVERTISED_LISTENERS_DOC = String.format("Specifies the listener addresses that the Kafka brokers will advertise to clients and other brokers." +
" The config is useful where the actual listener configuration <code>%s</code> does not represent the addresses that clients should" +
- " use to connect, such as in cloud environments. In environments using ZooKeeper, these addresses are published to ZooKeeper." +
+ " use to connect, such as in cloud environments." +
Review Comment:
Can we rephrase the next sentence?
For example:
```
The address would be published to and managed by the KRaft controller, the brokers pull these data from the controller as needed.
```
##########
server/src/main/java/org/apache/kafka/server/config/KRaftConfigs.java:
##########
@@ -36,29 +36,27 @@
public class KRaftConfigs {
/** KRaft mode configs */
public static final String PROCESS_ROLES_CONFIG = "process.roles";
- public static final String PROCESS_ROLES_DOC = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. " +
- "This configuration is only applicable for clusters in KRaft (Kafka Raft) mode (instead of ZooKeeper). Leave this config undefined or empty for ZooKeeper clusters.";
-
+ public static final String PROCESS_ROLES_DOC = "The roles that this process plays: 'broker', 'controller', or 'broker,controller' if it is both. ";
+
public static final String INITIAL_BROKER_REGISTRATION_TIMEOUT_MS_CONFIG = "initial.broker.registration.timeout.ms";
public static final int INITIAL_BROKER_REGISTRATION_TIMEOUT_MS_DEFAULT = 60000;
public static final String INITIAL_BROKER_REGISTRATION_TIMEOUT_MS_DOC = "When initially registering with the controller quorum, the number of milliseconds to wait before declaring failure and exiting the broker process.";
public static final String BROKER_HEARTBEAT_INTERVAL_MS_CONFIG = "broker.heartbeat.interval.ms";
public static final int BROKER_HEARTBEAT_INTERVAL_MS_DEFAULT = 2000;
- public static final String BROKER_HEARTBEAT_INTERVAL_MS_DOC = "The length of time in milliseconds between broker heartbeats. Used when running in KRaft mode.";
+ public static final String BROKER_HEARTBEAT_INTERVAL_MS_DOC = "The length of time in milliseconds between broker heartbeats.";
public static final String BROKER_SESSION_TIMEOUT_MS_CONFIG = "broker.session.timeout.ms";
public static final int BROKER_SESSION_TIMEOUT_MS_DEFAULT = 9000;
- public static final String BROKER_SESSION_TIMEOUT_MS_DOC = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made. Used when running in KRaft mode.";
+ public static final String BROKER_SESSION_TIMEOUT_MS_DOC = "The length of time in milliseconds that a broker lease lasts if no heartbeats are made.";
public static final String NODE_ID_CONFIG = "node.id";
public static final int EMPTY_NODE_ID = -1;
- public static final String NODE_ID_DOC = "The node ID associated with the roles this process is playing when <code>process.roles</code> is non-empty. " +
- "This is required configuration when running in KRaft mode.";
+ public static final String NODE_ID_DOC = "The node ID associated with the roles this process is playing when <code>process.roles</code> is non-empty.";
Review Comment:
I agree
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1929641799
##########
server-common/src/main/java/org/apache/kafka/server/config/QuotaConfig.java:
##########
@@ -54,9 +54,8 @@ public class QuotaConfig {
public static final String CLIENT_QUOTA_CALLBACK_CLASS_CONFIG = "client.quota.callback.class";
public static final String CLIENT_QUOTA_CALLBACK_CLASS_DOC = "The fully qualified name of a class that implements the ClientQuotaCallback interface, " +
- "which is used to determine quota limits applied to client requests. By default, the <user> and <client-id> " +
- "quotas that are stored in ZooKeeper are applied. For any given request, the most specific quota that matches the user principal " +
Review Comment:
I will open a PR to add it back.
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #18346:
URL: https://github.com/apache/kafka/pull/18346#issuecomment-2602957837
Applied to 4.0 too: https://github.com/apache/kafka/commit/9e80d7ed56439356d73846f479a91c7082e0b9c2
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #18346:
URL: https://github.com/apache/kafka/pull/18346#discussion_r1922522576
##########
server/src/main/java/org/apache/kafka/network/SocketServerConfigs.java:
##########
@@ -77,8 +77,8 @@ public class SocketServerConfigs {
public static final String ADVERTISED_LISTENERS_CONFIG = "advertised.listeners";
public static final String ADVERTISED_LISTENERS_DOC = String.format("Specifies the listener addresses that the Kafka brokers will advertise to clients and other brokers." +
" The config is useful where the actual listener configuration <code>%s</code> does not represent the addresses that clients should" +
- " use to connect, such as in cloud environments. In environments using ZooKeeper, these addresses are published to ZooKeeper." +
- " In Kraft mode, the address would be published to and managed by kraft controller, the brokers would pull these data from controller as needed." +
+ " use to connect, such as in cloud environments." +
+ " The address would be published to and managed by the KRaft controller, the brokers pull these data from the controller as needed." +
Review Comment:
`address` -> `addresses`
--
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
Re: [PR] KAFKA-18363 Clean up Server config documents which contained zookeeper [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18346:
URL: https://github.com/apache/kafka/pull/18346#issuecomment-2572216163
A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the `triage` label
should be removed to prevent this automation from happening again.
--
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