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 2022/09/19 04:52:23 UTC
[GitHub] [kafka] showuon opened a new pull request, #12660: MINOR: use addExact to avoid overflow and some cleanup
showuon opened a new pull request, #12660:
URL: https://github.com/apache/kafka/pull/12660
What changes in this PR:
1. Use `addExact` to avoid overflow in `BatchAccumulator#bytesNeeded`. We did use `addExact` in `bytesNeededForRecords` method, but forgot that when returning the result.
2. javadoc improvement
### 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 a diff in pull request #12660: MINOR: use addExact to avoid overflow and some cleanup
Posted by GitBox <gi...@apache.org>.
showuon commented on code in PR #12660:
URL: https://github.com/apache/kafka/pull/12660#discussion_r973861727
##########
raft/src/main/java/org/apache/kafka/raft/internals/BatchMemoryPool.java:
##########
@@ -96,9 +96,9 @@ public void release(ByteBuffer previouslyAllocated) {
try {
previouslyAllocated.clear();
- if (previouslyAllocated.limit() != batchSize) {
+ if (previouslyAllocated.capacity() != batchSize) {
throw new IllegalArgumentException("Released buffer with unexpected size "
- + previouslyAllocated.limit());
+ + previouslyAllocated.capacity());
Review Comment:
Although we cleared the buffer previously, the correct way to get the buffer size should be `Buffer#capacity`.
--
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 merged pull request #12660: MINOR: use addExact to avoid overflow and some cleanup
Posted by GitBox <gi...@apache.org>.
showuon merged PR #12660:
URL: https://github.com/apache/kafka/pull/12660
--
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 #12660: MINOR: use addExact to avoid overflow and some cleanup
Posted by GitBox <gi...@apache.org>.
showuon commented on PR #12660:
URL: https://github.com/apache/kafka/pull/12660#issuecomment-1250568976
@jsancio @hachikuji @dengziming , please take a look. 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 #12660: MINOR: use addExact to avoid overflow and some cleanup
Posted by GitBox <gi...@apache.org>.
showuon commented on PR #12660:
URL: https://github.com/apache/kafka/pull/12660#issuecomment-1254392246
Failed tests are unrelated:
```
Build / JDK 8 and Scala 2.12 / kafka.api.TransactionsTest.testBumpTransactionalEpoch(String).quorum=kraft
Build / JDK 17 and Scala 2.13 / kafka.network.ConnectionQuotasTest.testListenerConnectionRateLimitWhenActualRateAboveLimit()
Build / JDK 17 and Scala 2.13 / org.apache.kafka.streams.integration.EosIntegrationTest.shouldWriteLatestOffsetsToCheckpointOnShutdown[exactly_once_v2]
```
--
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 diff in pull request #12660: MINOR: use addExact to avoid overflow and some cleanup
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12660:
URL: https://github.com/apache/kafka/pull/12660#discussion_r977073273
##########
raft/src/main/java/org/apache/kafka/raft/internals/BatchMemoryPool.java:
##########
@@ -54,12 +54,12 @@ public BatchMemoryPool(int maxRetainedBatches, int batchSize) {
}
/**
- * Allocate a byte buffer in this pool.
+ * Allocate a byte buffer with {@code batchSize} in this pool.
*
* This method should always succeed and never return null. The sizeBytes parameter must be less than
* the batchSize used in the constructor.
*
- * @param sizeBytes is not used to determine the size of the byte buffer
Review Comment:
First time I've seen a field described in terms of what it is not 😉 .
--
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