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/12/19 07:01:03 UTC
[GitHub] [kafka] showuon opened a new pull request #11616: MINOR: update the description in example properties file
showuon opened a new pull request #11616:
URL: https://github.com/apache/kafka/pull/11616
When testing Kraft mode, I found the description for `controller.listener.names` is wrong and misleading. Fix the error, and update some other descriptions to make them clearer.
### 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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-997341888
@rondagostino @cmccabe , please help review. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771907372
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -712,7 +712,8 @@ object KafkaConfig {
"If it is not set, the metadata log is placed in the first log directory from log.dirs."
val MetadataSnapshotMaxNewRecordBytesDoc = "This is the maximum number of bytes in the log between the latest snapshot and the high-watermark needed before generating a new snapshot."
val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the controller. This is required " +
- "if running in KRaft mode. The ZK-based controller will not use this configuration."
+ "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 ZK-based controller should not set this configuration."
Review comment:
After https://github.com/apache/kafka/pull/11503, a ZooKeeper-based cluster that sets this config will fail to restart until this config is removed. We should note it in the doc, not just "ignore the config" as before. Besides, we should also have simple description explain we will use the 1st listener to communicating with the controller quorum. (ref: [KIP-631](https://cwiki.apache.org/confluence/display/KAFKA/KIP-631%3A+The+Quorum-based+Kafka+Controller))
Before
```
A comma-separated list of the names of the listeners used by the controller. This is required if running in KRaft mode. The ZK-based controller will not use this configuration.
```
After:
```
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.
Note: The ZK-based controller should not set this configuration.
```
--
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
[GitHub] [kafka] lakamsani commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
lakamsani commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-997454815
@showuon thanks for these updates. Very helpful. May I suggest a couple of more doc updates/clarifications in this area?
1. Wondering if it might be useful to add links to the example [broker only config file](https://github.com/apache/kafka/blob/trunk/config/kraft/broker.properties) and the example [controller only config file](https://github.com/apache/kafka/blob/trunk/config/kraft/controller.properties) that you reminded in [your email response](https://lists.apache.org/thread/11fh70rjzfpgwm3p3zhnzx1m1y23q82h) to this [section](https://github.com/apache/kafka/blob/trunk/config/kraft/README.md#process-roles) of the Kraft mode README file. I know they are sitting right there next to the sample `server.properties` but I was so focused on that file that I didn't pay much attention to the other two when I first email the thread about broker-only and controller-only samples.
2. Regarding the [node numbers discussion in this section](https://github.com/apache/kafka/tree/trunk/config/kraft#quorum-voters) of README, please clarify if the node numbering is global i.e in the 10 broker example, given that 1,2,3 are taken by controllers, the brokers can only be numbered from 4 onwards. Or is the numbering scoped to the type of the node. That is can we have node ids 1,2,3 for brokers only nodes and again numbers 1,2,3 for dual-mode nodes.
Excited about the simplicity and scalability (large # of topics, faster leader selection etc) that Kraft mode brings to Kafka and can't wait for production release. Thanks.
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r821648236
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
@dengziming , make sense. Could you update your finding in [KAFKA-13667](https://issues.apache.org/jira/browse/KAFKA-13667)? 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r803442548
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9092,CONTROLLER://:9093
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode.
Review comment:
Added one more line:
A comma-separated list of the names of the listeners used by the controller.
** If no explicit mapping set in `listener.security.protocol.map`, default will be using PLAINTEXT protocol**
This is required if running in KRaft mode.
--
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
[GitHub] [kafka] hachikuji commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806399392
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
How about this?
> Combined nodes (i.e. those with `process.roles=broker,controller`) must list the controller listener here at a minimum. If the broker listener is not defined, the default listener will use a host name will be equal to the value of `#java.net.InetAddress.getCanonicalHostName()`, with `PLAINTEXT` listener name, and port 9092.
By the way, I find this approach of allowing a default only for "broker" a bit messy. For a separate patch, but maybe we should require both listeners to be explicitly provided. What do you think?
##########
File path: config/kraft/controller.properties
##########
@@ -31,22 +31,17 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# Note that only the controller listeners are allowed here when `process.roles=controller`.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
-listeners=PLAINTEXT://:9093
+listeners=CONTROLLER://:9093
Review comment:
I wonder if it's helpful to have a note about consistency between this and `controller.quorum.voters`?
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning records; The default uses "sticky" partitioning logic, which attempts to fill batches sent to each broker in order to improve throughput.
Review comment:
Small improvement? I thought we should say something about balance.
> The default uses "sticky" partitioning logic which spreads the load evenly between partitions, but improves throughput by attempting to fill the batches sent to each partition.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806811337
##########
File path: config/kraft/controller.properties
##########
@@ -31,22 +31,17 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# Note that only the controller listeners are allowed here when `process.roles=controller`.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
-listeners=PLAINTEXT://:9093
+listeners=CONTROLLER://:9093
Review comment:
Good suggestion. Updated.
--
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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1040252753
@hachikuji , thanks for the comments. I've updated in this commit: https://github.com/apache/kafka/pull/11616/commits/acb5c31521a69835069e2dfd7a054393ad7abcc4. 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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771907372
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -712,7 +712,8 @@ object KafkaConfig {
"If it is not set, the metadata log is placed in the first log directory from log.dirs."
val MetadataSnapshotMaxNewRecordBytesDoc = "This is the maximum number of bytes in the log between the latest snapshot and the high-watermark needed before generating a new snapshot."
val ControllerListenerNamesDoc = "A comma-separated list of the names of the listeners used by the controller. This is required " +
- "if running in KRaft mode. The ZK-based controller will not use this configuration."
+ "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 ZK-based controller should not set this configuration."
Review comment:
After https://github.com/apache/kafka/pull/11503, a ZooKeeper-based cluster that sets this config will fail to restart until this config is removed. We should note it in the doc, not just "ignore the config" as before. Besides, we should also have simple description explain we will use the 1st listener to communicating with the controller quorum. (ref: [KIP-631](https://cwiki.apache.org/confluence/display/KAFKA/KIP-631%3A+The+Quorum-based+Kafka+Controller))
Before
`
A comma-separated list of the names of the listeners used by the controller. This is required if running in KRaft mode. The ZK-based controller will not use this configuration.
`
After:
`
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.`
`Note: The ZK-based controller should not set this configuration. `
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906523
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
Review comment:
Simplify it, to remove the last sentence since it's introduced in `listeners`
--
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
[GitHub] [kafka] hachikuji commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r801172935
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning events; default partitioner will try sticking to a partition until the batch is full
Review comment:
Do we use "events" consistently? Seems like "records" would be more in keeping with the APIs. For the latter half, how about this?
> The default uses "sticky" partitioning logic, which attempts to fill batches sent to each broker in order to improve throughput.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r772108297
##########
File path: config/kraft/README.md
##########
@@ -35,6 +35,9 @@ Formatting /tmp/kraft-combined-logs
If you are using multiple nodes, then you should run the format command on each node. Be sure to use the same cluster ID for each one.
+In this example, the `server.properties` file is a `broker,controller` combined node. If you want to have a separate `broker` and `controller` node,
+Please find the example properties file [here](https://github.com/apache/kafka/blob/trunk/config/kraft/broker.properties) and [here](https://github.com/apache/kafka/blob/trunk/config/kraft/controller.properties).
+
Review comment:
Added the last paragraph in `Format Storage Directories` section, to mention that there is also 2 other example properties file for reference.
--
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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-997637394
@lakamsani , thanks for your comments. I've updated the 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806812608
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
> For a separate patch, but maybe we should require both listeners to be explicitly provided. What do you think?
Agree. I'll file a jira for it later. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1041035737
Thanks for your patience, @hachikuji !
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906368
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
Review comment:
Make it clear about which default value will be used if not set.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906660
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning events; default partitioner will try sticking to a partition until the batch is full
#partitioner.class=
Review comment:
side fix: the default partitioner has changed to sticky partitioner.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r804339452
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
Review comment:
You are right! Update the description to:
// The address the socket server listens on.
**// This is required for combined role(i.e. process.roles=broker,controller) to configure the listeners for clients and controller**
// FORMAT:
// listeners = listener_name://host_name:port
// EXAMPLE:
// listeners = PLAINTEXT://your.host.name:9092
--
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
[GitHub] [kafka] hachikuji commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806399392
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
How about this?
> Combined nodes (i.e. those with `process.roles=broker,controller`) must list the controller listener here at a minimum. If the broker listener is not defined, the default listener will use a host name will be equal to the value of `#java.net.InetAddress.getCanonicalHostName()`, with `PLAINTEXT` listener name, and port 9092.
By the way, I find this approach of allowing a default only for "broker" a bit messy. For a separate patch, but maybe we should require both listeners to be explicitly provided. What do you think?
##########
File path: config/kraft/controller.properties
##########
@@ -31,22 +31,17 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# Note that only the controller listeners are allowed here when `process.roles=controller`.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
-listeners=PLAINTEXT://:9093
+listeners=CONTROLLER://:9093
Review comment:
I wonder if it's helpful to have a note about consistency between this and `controller.quorum.voters`?
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning records; The default uses "sticky" partitioning logic, which attempts to fill batches sent to each broker in order to improve throughput.
Review comment:
Small improvement? I thought we should say something about balance.
> The default uses "sticky" partitioning logic which spreads the load evenly between partitions, but improves throughput by attempting to fill the batches sent to each partition.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906571
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
Review comment:
Simplify it, to remove the last sentence since it's introduced in `listeners`
--
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
[GitHub] [kafka] rondagostino commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
rondagostino commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r784034488
##########
File path: config/kraft/README.md
##########
@@ -35,6 +35,9 @@ Formatting /tmp/kraft-combined-logs
If you are using multiple nodes, then you should run the format command on each node. Be sure to use the same cluster ID for each one.
+In this example, the `server.properties` file is a `broker,controller` combined node. If you want to have a separate `broker` and `controller` node,
Review comment:
s/a separate `broker` and `controller` node/separate `broker` and `controller` nodes/
##########
File path: config/kraft/README.md
##########
@@ -35,6 +35,9 @@ Formatting /tmp/kraft-combined-logs
If you are using multiple nodes, then you should run the format command on each node. Be sure to use the same cluster ID for each one.
+In this example, the `server.properties` file is a `broker,controller` combined node. If you want to have a separate `broker` and `controller` node,
+Please find the example properties file [here](https://github.com/apache/kafka/blob/trunk/config/kraft/broker.properties) and [here](https://github.com/apache/kafka/blob/trunk/config/kraft/controller.properties).
Review comment:
s/Please/please/
##########
File path: config/kraft/README.md
##########
@@ -93,7 +96,7 @@ controller.quorum.voters=1@controller1.example.com:9093,2@controller2.example.co
Each broker and each controller must set `controller.quorum.voters`. Note that the node ID supplied in the `controller.quorum.voters` configuration must match that supplied to the server.
So on controller1, node.id must be set to 1, and so forth. Note that there is no requirement for controller IDs to start at 0 or 1. However, the easiest and least confusing way to allocate
-node IDs is probably just to give each server a numeric ID, starting from 0.
+node IDs is probably just to give each server a numeric ID, starting from 0. Also note that the node ID is global and can not conflict with other node ID no matter what process role is.
Review comment:
s/Also note that the node ID is global and can not conflict with other node ID no matter what process role is./Also note that each node ID must be unique across all the nodes in a particular cluster; no two nodes can have the same node ID regardless of their `process.roles` values./
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
Review comment:
s/the host name will get from/the host name will be equal to the value of/
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode.
Review comment:
On a broker node (which this properties file corresponds to) any listener beyond the first one is ignored. So it would be good to either append a statement to that effect here.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r803449800
##########
File path: config/kraft/controller.properties
##########
@@ -39,10 +39,6 @@ controller.quorum.voters=1@localhost:9093
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9093
-# Listener name, hostname and port the broker will advertise to producers and consumers.
-# If not set, it uses the value for "listeners".
-#advertised.listeners=PLAINTEXT://your.host.name:9092
-
Review comment:
Remove this `advertised.listeners` section in controller-only role to avoid confusion, because there must be no advertised listeners when running just a KRaft controller. ref: [KAFKA-13456](https://issues.apache.org/jira/browse/KAFKA-13456)
--
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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1037199304
@hachikuji , thanks for the comments. I've updated the 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1034670296
@hachikuji , thanks for the comments. I've addressed most of them. Only 1 comment I'm not sure what you mean: https://github.com/apache/kafka/pull/11616#discussion_r801169378 . 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
[GitHub] [kafka] hachikuji merged pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji merged pull request #11616:
URL: https://github.com/apache/kafka/pull/11616
--
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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1037199304
@hachikuji , thanks for the comments. I've updated the 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji merged pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji merged pull request #11616:
URL: https://github.com/apache/kafka/pull/11616
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906368
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
Review comment:
Make it clear about which default value will be used if not set. (i.e. `PLAINTEXT://:9092`)
--
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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1013601840
@rondagostino , thanks for the comments. I've updated the PR in this commit: https://github.com/apache/kafka/pull/11616/commits/4b1363c01641a5f77d1f54b26286800818e98da9. Please take a look again. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dengziming commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
dengziming commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r821561398
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
> must list the controller listener here at a minimum.
It seems that we can't only list controller listener here in combined mode because `advertised.listeners` must be a subset of `listeners` and advertised listener can not be empty. so we must always set both controller and broker listeners here.
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
> must list the controller listener here at a minimum.
It seems that we can't only list controller listener here in combined mode because `advertised.listeners` must be a subset of `listeners` and advertised listener can not be empty. so we must always set both controller and broker listeners here.
--
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
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1012204127
@rondagostino @cmccabe , please help review. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r801162756
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
Review comment:
nit: I would just say "clients" since this also includes the Admin client.
##########
File path: config/kraft/README.md
##########
@@ -35,6 +35,9 @@ Formatting /tmp/kraft-combined-logs
If you are using multiple nodes, then you should run the format command on each node. Be sure to use the same cluster ID for each one.
+In this example, the `server.properties` file is a `broker,controller` combined node. If you want to separate `broker` and `controller` nodes,
Review comment:
How about this?
> This example configures the node as both a broker and controller (i.e. `process.roles=broker,controller`). It is also possible to run the broker and controller nodes separately. See [here](https://github.com/apache/kafka/blob/trunk/config/kraft/broker.properties) and [here](https://github.com/apache/kafka/blob/trunk/config/kraft/controller.properties) for example configurations.
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode. On a node with `process.roles=broker`, any listeners beyond the first one is ignored.
Review comment:
nit: instead of "any listeners beyond the first one is ignored," how about "only the first listed listener will be used by the broker"
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode. On a node with `process.roles=broker`, any listeners beyond the first one is ignored.
+# That is, you should only set 1 listener name here for the "broker" role node.
Review comment:
nit: this feels like overkill.
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning events; default partitioner will try sticking to a partition until the batch is full
Review comment:
Do we use "events" consistently? Seems like "records" would be more in keeping with the APIs.
> The default uses "sticky" partitioning logic, which attempts to fill batches in order to improve throughput.
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9092,CONTROLLER://:9093
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode.
Review comment:
It might be worth pointing out how that the controller listener is assumed to be PLAINTEXT as long as all other listeners are configured for PLAINTEXT as well.
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
Review comment:
Is this still accurate when process.roles=broker,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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806811337
##########
File path: config/kraft/controller.properties
##########
@@ -31,22 +31,17 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# Note that only the controller listeners are allowed here when `process.roles=controller`.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
-listeners=PLAINTEXT://:9093
+listeners=CONTROLLER://:9093
Review comment:
Good suggestion. Updated.
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning records; The default uses "sticky" partitioning logic, which attempts to fill batches sent to each broker in order to improve throughput.
Review comment:
a ha, you're right. I only focus on the comparison between sticky and round-robin partitioner. Updated.
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
> For a separate patch, but maybe we should require both listeners to be explicitly provided. What do you think?
Agree. I'll file a jira for it later. Thanks.
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
[KAFKA-13667](https://issues.apache.org/jira/browse/KAFKA-13667) is created for this improvement. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806812090
##########
File path: config/producer.properties
##########
@@ -23,7 +23,7 @@ bootstrap.servers=localhost:9092
# specify the compression codec for all data generated: none, gzip, snappy, lz4, zstd
compression.type=none
-# name of the partitioner class for partitioning events; default partition spreads data randomly
+# name of the partitioner class for partitioning records; The default uses "sticky" partitioning logic, which attempts to fill batches sent to each broker in order to improve throughput.
Review comment:
a ha, you're right. I only focus on the comparison between sticky and round-robin partitioner. Updated.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r806817526
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,24 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on.
+# This is required for combined role(i.e. process.roles=broker,controller) node to set the listeners at least for controller
Review comment:
[KAFKA-13667](https://issues.apache.org/jira/browse/KAFKA-13667) is created for this improvement. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#issuecomment-1040252753
@hachikuji , thanks for the comments. I've updated in this commit: https://github.com/apache/kafka/pull/11616/commits/acb5c31521a69835069e2dfd7a054393ad7abcc4. 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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r804339452
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
Review comment:
You are right! Update the description to:
// The address the socket server listens on.
**// This is required for combined role(i.e. process.roles=broker,controller) to set the listeners at least for controller**
// FORMAT:
...
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r805101525
##########
File path: config/kraft/controller.properties
##########
@@ -31,21 +31,16 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9093
Review comment:
Nice suggestion! It will make it work out-of-the-box when user adopt one broker.properties and another one controller.properties.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r805101777
##########
File path: config/kraft/controller.properties
##########
@@ -31,21 +31,16 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
Review comment:
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
[GitHub] [kafka] hachikuji commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r805070981
##########
File path: config/kraft/controller.properties
##########
@@ -31,21 +31,16 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9093
Review comment:
Could we use CONTROLLER as in broker.properties?
##########
File path: config/kraft/README.md
##########
@@ -35,6 +35,9 @@ Formatting /tmp/kraft-combined-logs
If you are using multiple nodes, then you should run the format command on each node. Be sure to use the same cluster ID for each one.
+This example configures the node as both a broker and controller (i.e. `process.roles=broker,controller`). It is also possible to run the broker and controller nodes separately.
+please see [here](https://github.com/apache/kafka/blob/trunk/config/kraft/broker.properties) and [here](https://github.com/apache/kafka/blob/trunk/config/kraft/controller.properties) for example configurations.
Review comment:
nit: capitalize "Please"
##########
File path: config/kraft/controller.properties
##########
@@ -31,21 +31,16 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
Review comment:
Is this another case that we need to fix? I think we need to have the controller listener here. Perhaps we should even say that _only_ the controller listeners are allowed here when `process.roles=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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r772108732
##########
File path: config/kraft/README.md
##########
@@ -93,7 +96,7 @@ controller.quorum.voters=1@controller1.example.com:9093,2@controller2.example.co
Each broker and each controller must set `controller.quorum.voters`. Note that the node ID supplied in the `controller.quorum.voters` configuration must match that supplied to the server.
So on controller1, node.id must be set to 1, and so forth. Note that there is no requirement for controller IDs to start at 0 or 1. However, the easiest and least confusing way to allocate
-node IDs is probably just to give each server a numeric ID, starting from 0.
+node IDs is probably just to give each server a numeric ID, starting from 0. Also note that the node ID is global and can not conflict with other node ID no matter what process role is.
Review comment:
Add a sentence to mention the node ID is global and can't conflict with each other no matter what process role is.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906602
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode.
controller.listener.names=CONTROLLER
Review comment:
Fix the wrong description for `controller.listener.names`
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r771906468
##########
File path: config/kraft/broker.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will get from
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://localhost:9092
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
Review comment:
It'd be better and clear to have an simple description for each config.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r803442548
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9092,CONTROLLER://:9093
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode.
Review comment:
Added one more line:
// A comma-separated list of the names of the listeners used by the controller.
** // If no explicit mapping set in `listener.security.protocol.map`, default will be using PLAINTEXT protocol**
// This is required if running in KRaft mode.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r803442548
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
+# java.net.InetAddress.getCanonicalHostName(), with PLAINTEXT listener name, and port 9092.
# FORMAT:
# listeners = listener_name://host_name:port
# EXAMPLE:
# listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9092,CONTROLLER://:9093
+
+# Name of listener used for communication between brokers.
inter.broker.listener.name=PLAINTEXT
-# Hostname and port the broker will advertise to producers and consumers. If not set,
-# it uses the value for "listeners" if configured. Otherwise, it will use the value
-# returned from java.net.InetAddress.getCanonicalHostName().
+# Listener name, hostname and port the broker will advertise to producers and consumers.
+# If not set, it uses the value for "listeners".
advertised.listeners=PLAINTEXT://localhost:9092
-# Listener, host name, and port for the controller to advertise to the brokers. If
-# this server is a controller, this listener must be configured.
+# A comma-separated list of the names of the listeners used by the controller.
+# This is required if running in KRaft mode.
Review comment:
Added one more line:
# A comma-separated list of the names of the listeners used by the controller.
**# If no explicit mapping set in `listener.security.protocol.map`, default will be using PLAINTEXT protocol**
# This is required if running in KRaft mode.
--
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
[GitHub] [kafka] showuon commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r803447044
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
Review comment:
Sorry, I don't get it. Will there be any difference for `process.roles=broker,controller`? Could you let me know. 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a change in pull request #11616: MINOR: update the description in Kraft example properties file
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11616:
URL: https://github.com/apache/kafka/pull/11616#discussion_r804270026
##########
File path: config/kraft/server.properties
##########
@@ -31,22 +31,23 @@ controller.quorum.voters=1@localhost:9093
############################# Socket Server Settings #############################
-# The address the socket server listens on. It will get the value returned from
-# java.net.InetAddress.getCanonicalHostName() if not configured.
+# The address the socket server listens on. If not configured, the host name will be equal to the value of
Review comment:
What I mean is whether it is allowable for `listeners` to be not configured when `process.roles=broker,controller`. At a minimum, don't we require the controller listener?
--
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