KAFKA-5032: Update the docs for message size configs across the board #3374

apurvam · 2017-06-19T21:23:16Z

Before 0.11, we used to have limits for maximum message size on the producer, broker, and consumer side.

From 0.11 onward, these limits apply to record batches as a whole. This patch updates the documentation of the configs to make this explicit.

A separate patch will have more extensive upgrade notes to tie all the changes together in one narrative.

apurvam · 2017-06-19T21:23:49Z

cc @hachikuji @ijuma

I chose not to mention explicit versions in the changes, because the documentation page is versioned over all.

asfgit · 2017-06-19T22:20:59Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/5481/
Test PASSed (JDK 7 and Scala 2.11).

asfgit · 2017-06-19T22:37:15Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/5466/
Test PASSed (JDK 8 and Scala 2.12).

hachikuji

Thanks for the patch. Left a few comments.

hachikuji · 2017-06-19T23:00:55Z

clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java

@@ -124,9 +124,9 @@
     */
    public static final String FETCH_MAX_BYTES_CONFIG = "fetch.max.bytes";
    private static final String FETCH_MAX_BYTES_DOC = "The maximum amount of data the server should return for a fetch request. " +


Maybe we should update MAX_PARTITION_FETCH_BYTES_DOC as well.

hachikuji · 2017-06-19T23:07:46Z

clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java

-            "This is not an absolute maximum, if the first message in the first non-empty partition of the fetch is larger than " +
-            "this value, the message will still be returned to ensure that the consumer can make progress. " +
-            "The maximum message size accepted by the broker is defined via <code>message.max.bytes</code> (broker config) or " +
+            "This is not an absolute maximum, if the first record batch in the first non-empty partition of the fetch is larger than " +


I think this is the only place in the consumer documentation where we mention the concept of a record batch. I wonder if we should offer a brief explanation. For example: "Records are fetched in batches by the consumer. If the first record batch in the first non-empty.... "

hachikuji · 2017-06-19T23:23:37Z

clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java

@@ -74,7 +74,7 @@
        "their data.";

    public static final String MAX_MESSAGE_BYTES_CONFIG = "max.message.bytes";
-    public static final String MAX_MESSAGE_BYTES_DOC = "This is largest message size Kafka will allow to be " +
+    public static final String MAX_MESSAGE_BYTES_DOC = "This is largest record batch size Kafka will allow to be " +


Hmm... Given the name of this config and the fact that we continue to support the v0/v1 formats, I wonder if we're ready to jump to the "record batch" language just yet. Maybe we could add something like this: "Messages are grouped in batches for efficiency. In previous releases, this applied only if compression was enabled, but it is generally true in the latest message format version. This configuration limits the maximum size of of a single uncompressed message for older formats and the size of a single message batch otherwise."

Yeah, the config name is annoying. I would document the current behaviour first and then have additional sentences for the older formats (and probably a brief reference to the unintuitive naming, which should come naturally after the older formats are explained).

Also, we should probably change KafkaConfig.MessageMaxBytesDoc to refer to this variable so that the docs remain consistent.

ijuma · 2017-06-20T12:50:05Z

clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java

@@ -109,8 +109,8 @@

    /** <code>max.request.size</code> */
    public static final String MAX_REQUEST_SIZE_CONFIG = "max.request.size";
-    private static final String MAX_REQUEST_SIZE_DOC = "The maximum size of a request in bytes. This is also effectively a cap on the maximum record size. Note that the server "
-                                                       + "has its own cap on record size which may be different from this. This setting will limit the number of record "
+    private static final String MAX_REQUEST_SIZE_DOC = "The maximum size of a request in bytes. This is also effectively a cap on the maximum record batch size. Note that the server "


Maybe we should move "This is also effectively a cap on ... which may be different from this." to after "This setting will limit ... sending huge requests.". It seems like the latter describes the purpose of the setting while the former is an additional implication.

ijuma · 2017-06-20T12:57:20Z

clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java

@@ -74,7 +74,7 @@
        "their data.";

    public static final String MAX_MESSAGE_BYTES_CONFIG = "max.message.bytes";
-    public static final String MAX_MESSAGE_BYTES_DOC = "This is largest message size Kafka will allow to be " +
+    public static final String MAX_MESSAGE_BYTES_DOC = "This is largest record batch size Kafka will allow to be " +


Yeah, the config name is annoying. I would document the current behaviour first and then have additional sentences for the older formats (and probably a brief reference to the unintuitive naming, which should come naturally after the older formats are explained).

Also, we should probably change KafkaConfig.MessageMaxBytesDoc to refer to this variable so that the docs remain consistent.

ijuma · 2017-06-20T12:59:25Z

core/src/main/scala/kafka/server/KafkaConfig.scala

@@ -542,7 +542,7 @@ object KafkaConfig {
  val ReplicaSocketTimeoutMsDoc = "The socket timeout for network requests. Its value should be at least replica.fetch.wait.max.ms"
  val ReplicaSocketReceiveBufferBytesDoc = "The socket receive buffer for network requests"
  val ReplicaFetchMaxBytesDoc = "The number of bytes of messages to attempt to fetch for each partition. This is not an absolute maximum, " +
-    "if the first message in the first non-empty partition of the fetch is larger than this value, the message will still be returned " +
+    "if the first record batch in the first non-empty partition of the fetch is larger than this value, the record batch will still be returned " +
    "to ensure that progress can be made. The maximum message size accepted by the broker is defined via " +


Do we want to update "The maximum message size.."?

We should also update ReplicaFetchResponseMaxBytesDoc.

… since everything is written and read in batches in the current version.

asfgit · 2017-06-22T23:21:33Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/5643/
Test PASSed (JDK 7 and Scala 2.11).

asfgit · 2017-06-22T23:26:20Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/5629/
Test PASSed (JDK 8 and Scala 2.12).

ijuma

Thanks for the updates, LGTM. I made a couple of small changes before merging to 0.11.0 and trunk. If @hachikuji has additional feedback, we can do that in a follow-up.

ijuma · 2017-06-27T13:57:04Z

core/src/main/scala/kafka/server/KafkaConfig.scala

@@ -416,7 +416,8 @@ object KafkaConfig {
  "start from " + MaxReservedBrokerIdProp + " + 1."
  val MessageMaxBytesDoc = "The maximum message size that the server can receive. Note that this limit also applies " +
    "to the total size of a compressed batch of messages (when compression is enabled). Additionally, in versions " +
-    "0.11 and later, all messages are written as batches and this setting applies to the total size of the batch."
+    "0.11 and later, all messages are written as batches and this setting applies to the total size of the batch. " +
+    "This can be set per topic with the topic level <code>max.message.bytes</code> config."


I changed this to refer to the TopicConfig definition concatenated with the last sentence before merging.

ijuma · 2017-06-27T13:58:02Z

clients/src/main/java/org/apache/kafka/common/config/TopicConfig.java

+        "they can fetch messages this large. </p> " +
+        "<p>Messages are grouped in batches for efficiency. In previous releases, this applied only if compression was " +
+        "enabled, but it is generally true in the latest message format version. This configuration limits the maximum " +
+        "size of of a single uncompressed message for older formats and the size of a single message batch otherwise. </p>";


I tweaked this a little before merging.

Before 0.11, we used to have limits for maximum message size on the producer, broker, and consumer side. From 0.11 onward, these limits apply to record batches as a whole. This patch updates the documentation of the configs to make this explicit. A separate patch will have more extensive upgrade notes to tie all the changes together in one narrative. Author: Apurva Mehta <apurva@confluent.io> Reviewers: Jason Gustafson <jason@confluent.io>, Ismael Juma <ismael@juma.me.uk> Closes #3374 from apurvam/KAFKA-5032-message-size-docs (cherry picked from commit f1cc800) Signed-off-by: Ismael Juma <ismael@juma.me.uk>

hachikuji reviewed Jun 19, 2017

View reviewed changes

ijuma reviewed Jun 20, 2017

View reviewed changes

Apurva Mehta added 2 commits June 22, 2017 15:20

Change references to 'message' in the size options to 'record batch',…

1cb351f

… since everything is written and read in batches in the current version.

Address PR Comments

c4b4f69

apurvam force-pushed the KAFKA-5032-message-size-docs branch from 8f404db to c4b4f69 Compare June 22, 2017 22:20

ijuma approved these changes Jun 27, 2017

View reviewed changes

asfgit closed this in f1cc800 Jun 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-5032: Update the docs for message size configs across the board #3374

KAFKA-5032: Update the docs for message size configs across the board #3374

apurvam commented Jun 19, 2017

apurvam commented Jun 19, 2017

asfgit commented Jun 19, 2017

asfgit commented Jun 19, 2017

hachikuji left a comment

hachikuji Jun 19, 2017

hachikuji Jun 19, 2017

hachikuji Jun 19, 2017

ijuma Jun 20, 2017

ijuma Jun 20, 2017

ijuma Jun 20, 2017

ijuma Jun 20, 2017

ijuma Jun 20, 2017

asfgit commented Jun 22, 2017

asfgit commented Jun 22, 2017

ijuma left a comment

ijuma Jun 27, 2017

ijuma Jun 27, 2017

KAFKA-5032: Update the docs for message size configs across the board #3374

KAFKA-5032: Update the docs for message size configs across the board #3374

Conversation

apurvam commented Jun 19, 2017

apurvam commented Jun 19, 2017

asfgit commented Jun 19, 2017

asfgit commented Jun 19, 2017

hachikuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Jun 22, 2017

asfgit commented Jun 22, 2017

ijuma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment