You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "jayteej (via GitHub)" <gi...@apache.org> on 2024/12/06 17:17:06 UTC
[PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
jayteej opened a new pull request, #18080:
URL: https://github.com/apache/kafka/pull/18080
KAFKA-16368: As part of KIP-1030 we are increasing the default linger.ms to 5ms. System tests updated and behaviour validated manually.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1894812224
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -147,8 +148,9 @@ public class ProducerConfig extends AbstractConfig {
+ "of as analogous to Nagle's algorithm in TCP. This setting gives the upper bound on the delay for batching: once "
+ "we get <code>" + BATCH_SIZE_CONFIG + "</code> worth of records for a partition it will be sent immediately regardless of this "
+ "setting, however if we have fewer than this many bytes accumulated for this partition we will 'linger' for the "
- + "specified time waiting for more records to show up. This setting defaults to 0 (i.e. no delay). Setting <code>" + LINGER_MS_CONFIG + "=5</code>, "
- + "for example, would have the effect of reducing the number of requests sent but would add up to 5ms of latency to records sent in the absence of load.";
+ + "specified time waiting for more records to show up. This setting defaults to 5 (i.e. 5ms delay). Increasing <code>" + LINGER_MS_CONFIG + "=50</code>, "
+ + "for example, would have the effect of reducing the number of requests sent but would add up to 50ms of latency to records sent in the absence of load."
Review Comment:
I don't follow, we seem to be saying that increasing from 5ms to 50ms would add up to 50ms of latency?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "jayteej (via GitHub)" <gi...@apache.org>.
jayteej commented on PR #18080:
URL: https://github.com/apache/kafka/pull/18080#issuecomment-2531867361
@divijvaidya done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya commented on PR #18080:
URL: https://github.com/apache/kafka/pull/18080#issuecomment-2568218711
We pushed out a commit https://github.com/apache/kafka/commit/e099fce567094b1143ded8516ffd748b5425667f two weeks ago which should make the `testBumpTransactionalEpochWithTV2Enabled` stable. This is the test which is failing in this build.
@jayteej can you please rebase from trunk one more time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya merged PR #18080:
URL: https://github.com/apache/kafka/pull/18080
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "jayteej (via GitHub)" <gi...@apache.org>.
jayteej commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1895864442
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -147,8 +148,9 @@ public class ProducerConfig extends AbstractConfig {
+ "of as analogous to Nagle's algorithm in TCP. This setting gives the upper bound on the delay for batching: once "
+ "we get <code>" + BATCH_SIZE_CONFIG + "</code> worth of records for a partition it will be sent immediately regardless of this "
+ "setting, however if we have fewer than this many bytes accumulated for this partition we will 'linger' for the "
- + "specified time waiting for more records to show up. This setting defaults to 0 (i.e. no delay). Setting <code>" + LINGER_MS_CONFIG + "=5</code>, "
- + "for example, would have the effect of reducing the number of requests sent but would add up to 5ms of latency to records sent in the absence of load.";
+ + "specified time waiting for more records to show up. This setting defaults to 5 (i.e. 5ms delay). Increasing <code>" + LINGER_MS_CONFIG + "=50</code>, "
+ + "for example, would have the effect of reducing the number of requests sent but would add up to 50ms of latency to records sent in the absence of load."
Review Comment:
I have not changed the wording here. This is the same example with different values. I can change the wording also if you think it helps or isn't grokable?
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -93,8 +93,9 @@ public class ProducerConfig extends AbstractConfig {
+ "<p>"
+ "Note: This setting gives the upper bound of the batch size to be sent. If we have fewer than this many bytes accumulated "
+ "for this partition, we will 'linger' for the <code>linger.ms</code> time waiting for more records to show up. "
- + "This <code>linger.ms</code> setting defaults to 0, which means we'll immediately send out a record even the accumulated "
- + "batch size is under this <code>batch.size</code> setting.";
+ + "This <code>linger.ms</code> setting defaults to 5, which means we'll wait 5ms for the accumulated "
+ + "<code>batch.size</code> to fill as much as it can in this timeframe. This value previously defaulted"
+ + "to 0, but over time we have observed that the IO overhead caused by smaller batches negates any latency gains.";
Review Comment:
Will update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1894810882
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -93,8 +93,9 @@ public class ProducerConfig extends AbstractConfig {
+ "<p>"
+ "Note: This setting gives the upper bound of the batch size to be sent. If we have fewer than this many bytes accumulated "
+ "for this partition, we will 'linger' for the <code>linger.ms</code> time waiting for more records to show up. "
- + "This <code>linger.ms</code> setting defaults to 0, which means we'll immediately send out a record even the accumulated "
- + "batch size is under this <code>batch.size</code> setting.";
+ + "This <code>linger.ms</code> setting defaults to 5, which means we'll wait 5ms for the accumulated "
+ + "<code>batch.size</code> to fill as much as it can in this timeframe. This value previously defaulted"
+ + "to 0, but over time we have observed that the IO overhead caused by smaller batches negates any latency gains.";
Review Comment:
The wording here is a little odd. I would write something like:
"This defaults to 5, which means the producer will wait for 5ms or until the record batch is of <code>batch.size</code> (whichever happens first) before sending the record batch. Note that broker backpressure can result in the effective linger time to be higher than this setting.
The default changed from <code>0</code> to <code>5</code> in Apache Kafka 4.0 as the efficiency gains from larger batches typically result in similar or lower producer latency despite the increased linger."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "jayteej (via GitHub)" <gi...@apache.org>.
jayteej commented on PR #18080:
URL: https://github.com/apache/kafka/pull/18080#issuecomment-2541282331
These tests are passing locally so I'm not sure whats going on. Will try another rebase to trunk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1894810882
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -93,8 +93,9 @@ public class ProducerConfig extends AbstractConfig {
+ "<p>"
+ "Note: This setting gives the upper bound of the batch size to be sent. If we have fewer than this many bytes accumulated "
+ "for this partition, we will 'linger' for the <code>linger.ms</code> time waiting for more records to show up. "
- + "This <code>linger.ms</code> setting defaults to 0, which means we'll immediately send out a record even the accumulated "
- + "batch size is under this <code>batch.size</code> setting.";
+ + "This <code>linger.ms</code> setting defaults to 5, which means we'll wait 5ms for the accumulated "
+ + "<code>batch.size</code> to fill as much as it can in this timeframe. This value previously defaulted"
+ + "to 0, but over time we have observed that the IO overhead caused by smaller batches negates any latency gains.";
Review Comment:
The wording here is a little odd. I would write something like:
"This defaults to 5, which means the producer will wait for 5ms or until the record batch is of <code>batch.size</code> (whichever happens first) before sending the record batch. Note that broker backpressure can result in a higher effective linger time than this setting.
The default changed from <code>0</code> to <code>5</code> in Apache Kafka 4.0 as the efficiency gains from larger batches typically result in similar or lower producer latency despite the increased linger."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya commented on PR #18080:
URL: https://github.com/apache/kafka/pull/18080#issuecomment-2531684699
Similar comment as https://github.com/apache/kafka/pull/18106#issuecomment-2529514526. We need to be very explicit about the impact customers can expect on upgrade for this particular 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
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "jayteej (via GitHub)" <gi...@apache.org>.
jayteej commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1895995008
##########
docs/upgrade.html:
##########
@@ -175,6 +175,9 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
</li>
<li>The deprecated <code>sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata>, String)</code> method has been removed from the Producer API.
</li>
+ <li>The default value of <code>linger.ms</code> has been changed from 0ms to 5ms. The impact of this is larger request batches will be filled by default at the expense of
+ time in the producer. It is generally observed that values lower than 5ms do not yield any latency benefits due to the IO overhead of smaller requests.
Review Comment:
Done
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -93,8 +93,9 @@ public class ProducerConfig extends AbstractConfig {
+ "<p>"
+ "Note: This setting gives the upper bound of the batch size to be sent. If we have fewer than this many bytes accumulated "
+ "for this partition, we will 'linger' for the <code>linger.ms</code> time waiting for more records to show up. "
- + "This <code>linger.ms</code> setting defaults to 0, which means we'll immediately send out a record even the accumulated "
- + "batch size is under this <code>batch.size</code> setting.";
+ + "This <code>linger.ms</code> setting defaults to 5, which means we'll wait 5ms for the accumulated "
+ + "<code>batch.size</code> to fill as much as it can in this timeframe. This value previously defaulted"
+ + "to 0, but over time we have observed that the IO overhead caused by smaller batches negates any latency gains.";
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "jayteej (via GitHub)" <gi...@apache.org>.
jayteej commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1895864718
##########
docs/upgrade.html:
##########
@@ -175,6 +175,9 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
</li>
<li>The deprecated <code>sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata>, String)</code> method has been removed from the Producer API.
</li>
+ <li>The default value of <code>linger.ms</code> has been changed from 0ms to 5ms. The impact of this is larger request batches will be filled by default at the expense of
+ time in the producer. It is generally observed that values lower than 5ms do not yield any latency benefits due to the IO overhead of smaller requests.
Review Comment:
Will update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18080:
URL: https://github.com/apache/kafka/pull/18080#discussion_r1894812355
##########
docs/upgrade.html:
##########
@@ -175,6 +175,9 @@ <h5><a id="upgrade_400_notable" href="#upgrade_400_notable">Notable changes in 4
</li>
<li>The deprecated <code>sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata>, String)</code> method has been removed from the Producer API.
</li>
+ <li>The default value of <code>linger.ms</code> has been changed from 0ms to 5ms. The impact of this is larger request batches will be filled by default at the expense of
+ time in the producer. It is generally observed that values lower than 5ms do not yield any latency benefits due to the IO overhead of smaller requests.
Review Comment:
I would probably use similar wording to the one I suggested in a different comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16368: Update default linger.ms to 5 for KIP-1030 [kafka]
Posted by "jayteej (via GitHub)" <gi...@apache.org>.
jayteej commented on PR #18080:
URL: https://github.com/apache/kafka/pull/18080#issuecomment-2592977515
Known flakey failure
```
./gradlew clean :clients:test --tests 'org.apache.kafka.clients.ClientUtilsTest'
Gradle Test Run :clients:test > Gradle Test Executor 47 > ClientUtilsTest > testParseAndValidateAddressesWithReverseLookup() FAILED
org.opentest4j.AssertionFailedError: Unexpected addresses [a96-7-128-175.deploy.static.akamaitechnologies.com, a23-215-0-138.deploy.static.akamaitechnologies.com, a23-215-0-136.deploy.static.akamaitechnologies.com, a23-192-228-84.deploy.static.akamaitechnologies.com, a23-192-228-80.deploy.static.akamaitechnologies.com, a96-7-128-198.deploy.static.akamaitechnologies.com, 2600:1408:ec00:36:0:0:1736:7f24, 2600:1406:bc00:53:0:0:b81e:94ce, 2600:1406:bc00:53:0:0:b81e:94c8, 2600:1406:3a00:21:0:0:173e:2e66, 2600:1406:3a00:21:0:0:173e:2e65, 2600:1408:ec00:36:0:0:1736:7f31] ==> expected: <true> but was: <false>
at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214)
at app//org.apache.kafka.clients.ClientUtilsTest.testParseAndValidateAddressesWithReverseLookup(ClientUtilsTest.java:67)
```
--
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