[GitHub] [kafka] chia7712 commented on a change in pull request #10535: MINOR: Remove duplicate method in test classes
chia7712 commented on a change in pull request #10535:
URL: https://github.com/apache/kafka/pull/10535#discussion_r624639802
##
File path: core/src/test/scala/unit/kafka/server/BaseRequestTest.scala
##
@@ -136,7 +135,7 @@ abstract class BaseRequestTest extends
IntegrationTestHarness {
}
def sendWithHeader(request: AbstractRequest, header: RequestHeader, socket:
Socket): Unit = {
-val serializedBytes =
Utils.toArray(RequestTestUtils.serializeRequestWithHeader(header, request))
+val serializedBytes = Utils.toArray(request.serializeWithHeader(header))
sendRequest(socket, serializedBytes)
Review comment:
It seems this method can be replaced by
`IntegrationTestUtils.sendRequest`
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] wenbingshen commented on pull request #10617: KAFKA-12734: LazyTimeIndex & LazyOffsetIndex may cause niobufferoverflow when skip activeSegment sanityCheck
wenbingshen commented on pull request #10617: URL: https://github.com/apache/kafka/pull/10617#issuecomment-830757283 @junrao I did some thinking again today. The timeindex problems of [KAFKA-10471](https://issues.apache.org/jira/browse/KAFKA-10471) and [KAFKA-12734](https://issues.apache.org/jira/browse/KAFKA-12734) all come from `reset the index size of the currently active log segment to allow more entries`. Compared with directly deleting the cleanShutDownFiles file, does the impact of only detecting the index file of the active segment have less impact? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Resolved] (KAFKA-12661) ConfigEntry#equal does not compare other fields when value is NOT null
[
https://issues.apache.org/jira/browse/KAFKA-12661?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chia-Ping Tsai resolved KAFKA-12661.
Fix Version/s: 3.0.0
Resolution: Fixed
> ConfigEntry#equal does not compare other fields when value is NOT null
> ---
>
> Key: KAFKA-12661
> URL: https://issues.apache.org/jira/browse/KAFKA-12661
> Project: Kafka
> Issue Type: Bug
>Reporter: Chia-Ping Tsai
>Assignee: Chia-Ping Tsai
>Priority: Minor
> Fix For: 3.0.0
>
>
> {code:java}
> return this.name.equals(that.name) &&
> this.value != null ? this.value.equals(that.value) :
> that.value == null &&
> this.isSensitive == that.isSensitive &&
> this.isReadOnly == that.isReadOnly &&
> this.source == that.source &&
> Objects.equals(this.synonyms, that.synonyms);
> {code}
> the second value of ternary operator is "that.value == null &&
> this.isSensitive == that.isSensitive &&
> this.isReadOnly == that.isReadOnly &&
> this.source == that.source &&
> Objects.equals(this.synonyms, that.synonyms);" rather than
> "that.value == null". Hence, it does not compare other fields when value is
> not null.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] chia7712 merged pull request #10446: KAFKA-12661 ConfigEntry#equal does not compare other fields when value is NOT null
chia7712 merged pull request #10446: URL: https://github.com/apache/kafka/pull/10446 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #10446: KAFKA-12661 ConfigEntry#equal does not compare other fields when value is NOT null
chia7712 commented on pull request #10446: URL: https://github.com/apache/kafka/pull/10446#issuecomment-830741353 ``` Build / JDK 8 and Scala 2.12 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopicsWithManyPartitions() ``` unrelated error. Merge this PR 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #10588: KAFKA-12662: add unit test for ProducerPerformance
tang7526 commented on pull request #10588: URL: https://github.com/apache/kafka/pull/10588#issuecomment-830735655 > @tang7526 , thanks for the patch! Could we add some tests for the `argParser`? ex: try pass unexpected arguments and verify the error thrown. Thanks. @showuon Thank you for your opinion. I have added a test for `argParser`. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Comment Edited] (KAFKA-12605) kafka consumer churns through buffer memory iterating over records
[
https://issues.apache.org/jira/browse/KAFKA-12605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337897#comment-17337897
]
radai rosenblatt edited comment on KAFKA-12605 at 5/2/21, 12:37 AM:
PR filed against trunk - [https://github.com/apache/kafka/pull/10624]
was (Author: radai):
PR files against trunk - https://github.com/apache/kafka/pull/10624
> kafka consumer churns through buffer memory iterating over records
> --
>
> Key: KAFKA-12605
> URL: https://issues.apache.org/jira/browse/KAFKA-12605
> Project: Kafka
> Issue Type: Improvement
>Affects Versions: 2.7.0
>Reporter: radai rosenblatt
>Priority: Major
> Attachments: Screen Shot 2021-04-01 at 3.55.47 PM.png
>
>
> we recently conducted analysis on memory allocations by the kafka consumer
> and found a significant amount of buffers that graduate out of the young gen
> causing GC load.
>
> these are tthe buffers used to gunzip record batches in the consumer when
> polling. since the same iterator (and underlying streams and buffers) are
> likely to live through several poll() cycles these buffers graduate out of
> young gen and cause issues.
>
> see attached memory allocation flame graph:
> !Screen Shot 2021-04-01 at 3.55.47 PM.png!
> the code causing this is in CompressionTypye.GZIP (taken from current trunk):
> {code:java}
> @Override
> public InputStream wrapForInput(ByteBuffer buffer, byte messageVersion,
> BufferSupplier decompressionBufferSupplier) {
> try {
> // Set output buffer (uncompressed) to 16 KB (none by default) and
> input buffer (compressed) to
> // 8 KB (0.5 KB by default) to ensure reasonable performance in cases
> where the caller reads a small
> // number of bytes (potentially a single byte)
> return new BufferedInputStream(new GZIPInputStream(new
> ByteBufferInputStream(buffer), 8 * 1024),
> 16 * 1024);
> } catch (Exception e) {
> throw new KafkaException(e);
> }
> }{code}
> it allocated 2 buffers - 8K and 16K even though a BufferSupplier is available
> to attempt re-use.
>
> i believe it is possible to actually get both tthose buffers from the
> supplier, and return them when iteration over the record batch is done.
> doing so will require subclassing BufferedInputStream and GZIPInputStream
> (or its parent class) to allow supplying external buffers onto them. also
> some lifecycle hook would be needed to return said buffers to the pool when
> iteration is done.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (KAFKA-12605) kafka consumer churns through buffer memory iterating over records
[
https://issues.apache.org/jira/browse/KAFKA-12605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337897#comment-17337897
]
radai rosenblatt commented on KAFKA-12605:
--
PR files against trunk - https://github.com/apache/kafka/pull/10624
> kafka consumer churns through buffer memory iterating over records
> --
>
> Key: KAFKA-12605
> URL: https://issues.apache.org/jira/browse/KAFKA-12605
> Project: Kafka
> Issue Type: Improvement
>Affects Versions: 2.7.0
>Reporter: radai rosenblatt
>Priority: Major
> Attachments: Screen Shot 2021-04-01 at 3.55.47 PM.png
>
>
> we recently conducted analysis on memory allocations by the kafka consumer
> and found a significant amount of buffers that graduate out of the young gen
> causing GC load.
>
> these are tthe buffers used to gunzip record batches in the consumer when
> polling. since the same iterator (and underlying streams and buffers) are
> likely to live through several poll() cycles these buffers graduate out of
> young gen and cause issues.
>
> see attached memory allocation flame graph:
> !Screen Shot 2021-04-01 at 3.55.47 PM.png!
> the code causing this is in CompressionTypye.GZIP (taken from current trunk):
> {code:java}
> @Override
> public InputStream wrapForInput(ByteBuffer buffer, byte messageVersion,
> BufferSupplier decompressionBufferSupplier) {
> try {
> // Set output buffer (uncompressed) to 16 KB (none by default) and
> input buffer (compressed) to
> // 8 KB (0.5 KB by default) to ensure reasonable performance in cases
> where the caller reads a small
> // number of bytes (potentially a single byte)
> return new BufferedInputStream(new GZIPInputStream(new
> ByteBufferInputStream(buffer), 8 * 1024),
> 16 * 1024);
> } catch (Exception e) {
> throw new KafkaException(e);
> }
> }{code}
> it allocated 2 buffers - 8K and 16K even though a BufferSupplier is available
> to attempt re-use.
>
> i believe it is possible to actually get both tthose buffers from the
> supplier, and return them when iteration over the record batch is done.
> doing so will require subclassing BufferedInputStream and GZIPInputStream
> (or its parent class) to allow supplying external buffers onto them. also
> some lifecycle hook would be needed to return said buffers to the pool when
> iteration is done.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] radai-rosenblatt opened a new pull request #10624: KAFKA-12605 - make GZIP decompression use BufferSupplier
radai-rosenblatt opened a new pull request #10624: URL: https://github.com/apache/kafka/pull/10624 as laid out in https://issues.apache.org/jira/browse/KAFKA-12605 kafka consumers decoding gzip'ed payloads currently do not re-use memory buffers because the JDK classes used have no support for it. this PR adds buffer reuse support to gzip decoding. unfortunately, since the JDK classes involved are not properly extensible I've had to make copies of them. modification to these copies are kept minimal: 1. buffers now come from, and are returned to, suppliers 2. some use of Unsafe has been replaced with more portable code 3. minor changes required to comply with kafka's checkstyle 4. KafkaBufferedInputStream does not fully support the mark() operation as that may involve buffer re-allocation. I have not found any usage of mark() in kafka code though. so far only decompression is supported. compression may be added later. a (randomized) test has been added that I have run on my machine for several hours with no issues. ### 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (KAFKA-12635) Mirrormaker 2 offset sync is incorrect if the target partition is empty
[ https://issues.apache.org/jira/browse/KAFKA-12635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337896#comment-17337896 ] Ning Zhang commented on KAFKA-12635: > This state can be reached if the source consumer group consumed some records >that were now deleted (like by a retention policy), or if Mirrormaker >replication is set to start at "latest". [~fyi] I am reading your above statement several times, and could not figure out the reproduce scenario. Do you mind to share the scenario step-by-step from a good state to a bad state? Thanks > Mirrormaker 2 offset sync is incorrect if the target partition is empty > --- > > Key: KAFKA-12635 > URL: https://issues.apache.org/jira/browse/KAFKA-12635 > Project: Kafka > Issue Type: Bug > Components: mirrormaker >Affects Versions: 2.7.0 >Reporter: Frank Yi >Assignee: Ning Zhang >Priority: Major > > This bug occurs when using Mirrormaker with "sync.group.offsets.enabled = > true". > If a source partition is empty, but the source consumer group's offset for > that partition is non-zero, then Mirrormaker sets the target consumer group's > offset for that partition to the literal, not translated, offset of the > source consumer group. This state can be reached if the source consumer group > consumed some records that were now deleted (like by a retention policy), or > if Mirrormaker replication is set to start at "latest". This bug causes the > target consumer group's lag for that partition to be negative and breaks > offset sync for that partition until lag is positive. > The correct behavior when the source partition is empty would be to set the > target offset to the translated offset, not literal offset, which in this > case would always be 0. > Original email thread on this issue: > https://lists.apache.org/thread.html/r7c54ee5f57227367b911d4abffa72781772d8dd3b72d75eb65ee19f7%40%3Cusers.kafka.apache.org%3E -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-12635) Mirrormaker 2 offset sync is incorrect if the target partition is empty
[ https://issues.apache.org/jira/browse/KAFKA-12635?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17337894#comment-17337894 ] Ning Zhang commented on KAFKA-12635: hi [~dragotic] could you please elaborate on how you can re-produce the issue step-by-step? > Mirrormaker 2 offset sync is incorrect if the target partition is empty > --- > > Key: KAFKA-12635 > URL: https://issues.apache.org/jira/browse/KAFKA-12635 > Project: Kafka > Issue Type: Bug > Components: mirrormaker >Affects Versions: 2.7.0 >Reporter: Frank Yi >Assignee: Ning Zhang >Priority: Major > > This bug occurs when using Mirrormaker with "sync.group.offsets.enabled = > true". > If a source partition is empty, but the source consumer group's offset for > that partition is non-zero, then Mirrormaker sets the target consumer group's > offset for that partition to the literal, not translated, offset of the > source consumer group. This state can be reached if the source consumer group > consumed some records that were now deleted (like by a retention policy), or > if Mirrormaker replication is set to start at "latest". This bug causes the > target consumer group's lag for that partition to be negative and breaks > offset sync for that partition until lag is positive. > The correct behavior when the source partition is empty would be to set the > target offset to the translated offset, not literal offset, which in this > case would always be 0. > Original email thread on this issue: > https://lists.apache.org/thread.html/r7c54ee5f57227367b911d4abffa72781772d8dd3b72d75eb65ee19f7%40%3Cusers.kafka.apache.org%3E -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] wenbingshen opened a new pull request #10623: MINOR: Clean up some redundant code from ReplicaManager
wenbingshen opened a new pull request #10623: URL: https://github.com/apache/kafka/pull/10623 1. brokerEpoch has been processed in handleStopReplicaRequest, and has no practical effect in ReplicaManager.stopReplicas. 2. responseMap.put(topicPartition, Errors.forException(e)) has a redundant put in stopReplicas. ### 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Resolved] (KAFKA-12154) API and implementation for snapshot loading
[
https://issues.apache.org/jira/browse/KAFKA-12154?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Gustafson resolved KAFKA-12154.
-
Resolution: Fixed
> API and implementation for snapshot loading
> ---
>
> Key: KAFKA-12154
> URL: https://issues.apache.org/jira/browse/KAFKA-12154
> Project: Kafka
> Issue Type: Sub-task
> Components: replication
>Reporter: Jose Armando Garcia Sancio
>Assignee: Jose Armando Garcia Sancio
>Priority: Major
>
> Following changes need to be implemented:
> 1. Notify the {{Listener}} when a snapshot is available:
> {code:java}
> interface Listener {
> void handleSnapshot(SnapshotReader snapshot);
> ...
> } {code}
> It is possible that we can reuse the existing {{BatchReader}} but I am not
> sure at this point. Whenever {{handleSnapshot}} is called the implementation
> should assume that the state has been rewound.
>
> 2. Update the {{ListenerContext}} when a snapshot is available:
> {code:java}
> class ListenerContext {
> void fireHandleSnapshot(SnapshotReader snapshot);
> }{code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] hachikuji merged pull request #10085: KAFKA-12154: Snapshot Loading API
hachikuji merged pull request #10085: URL: https://github.com/apache/kafka/pull/10085 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10620: KAFKA-12736: KafkaProducer.flush holds onto completed ProducerBatch(s) until flush completes
ijuma commented on a change in pull request #10620:
URL: https://github.com/apache/kafka/pull/10620#discussion_r624529198
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
##
@@ -710,8 +710,11 @@ private boolean appendsInProgress() {
*/
public void awaitFlushCompletion() throws InterruptedException {
try {
-for (ProducerBatch batch : this.incomplete.copyAll())
-batch.produceFuture.await();
+// Make a copy of of the request results at the time the flush is
called.
+// We avoid making a copy of the full incomplete batch collections
to allow
+// garbage collection.
Review comment:
That makes sense. It's worth clarifying that 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] lbradstreet commented on a change in pull request #10620: KAFKA-12736: KafkaProducer.flush holds onto completed ProducerBatch(s) until flush completes
lbradstreet commented on a change in pull request #10620:
URL: https://github.com/apache/kafka/pull/10620#discussion_r624525594
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
##
@@ -710,8 +710,11 @@ private boolean appendsInProgress() {
*/
public void awaitFlushCompletion() throws InterruptedException {
try {
-for (ProducerBatch batch : this.incomplete.copyAll())
-batch.produceFuture.await();
+// Make a copy of of the request results at the time the flush is
called.
+// We avoid making a copy of the full incomplete batch collections
to allow
+// garbage collection.
Review comment:
We do have a reference to incomplete, however the sender will remove the
producer batches from the original incomplete collection. It's not currently
freeing the individual ProducerBatch(s) because we're making a full copy of
incomplete's contents.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] lbradstreet commented on a change in pull request #10620: KAFKA-12736: KafkaProducer.flush holds onto completed ProducerBatch(s) until flush completes
lbradstreet commented on a change in pull request #10620:
URL: https://github.com/apache/kafka/pull/10620#discussion_r624525594
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
##
@@ -710,8 +710,11 @@ private boolean appendsInProgress() {
*/
public void awaitFlushCompletion() throws InterruptedException {
try {
-for (ProducerBatch batch : this.incomplete.copyAll())
-batch.produceFuture.await();
+// Make a copy of of the request results at the time the flush is
called.
+// We avoid making a copy of the full incomplete batch collections
to allow
+// garbage collection.
Review comment:
We do have a reference to incomplete, however the sender will remove the
producer batches from the original incomplete collection. It's not currently
doing so because we're making a full copy of incomplete's contents.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Resolved] (KAFKA-12683) Remove deprecated "UsePreviousTimeOnInvalidTimeStamp"
[ https://issues.apache.org/jira/browse/KAFKA-12683?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Guozhang Wang resolved KAFKA-12683. --- Fix Version/s: 3.0.0 Assignee: Guozhang Wang Resolution: Fixed > Remove deprecated "UsePreviousTimeOnInvalidTimeStamp" > - > > Key: KAFKA-12683 > URL: https://issues.apache.org/jira/browse/KAFKA-12683 > Project: Kafka > Issue Type: Sub-task >Reporter: Guozhang Wang >Assignee: Guozhang Wang >Priority: Major > Fix For: 3.0.0 > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] guozhangwang merged pull request #10622: MINOR: Remove unused Utils.delete
guozhangwang merged pull request #10622: URL: https://github.com/apache/kafka/pull/10622 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] guozhangwang merged pull request #10557: KAFKA-12683: Remove deprecated UsePreviousTimeOnInvalidTimestamp
guozhangwang merged pull request #10557: URL: https://github.com/apache/kafka/pull/10557 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] lbradstreet commented on a change in pull request #10620: KAFKA-12736: KafkaProducer.flush holds onto completed ProducerBatch(s) until flush completes
lbradstreet commented on a change in pull request #10620:
URL: https://github.com/apache/kafka/pull/10620#discussion_r624525594
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
##
@@ -710,8 +710,11 @@ private boolean appendsInProgress() {
*/
public void awaitFlushCompletion() throws InterruptedException {
try {
-for (ProducerBatch batch : this.incomplete.copyAll())
-batch.produceFuture.await();
+// Make a copy of of the request results at the time the flush is
called.
+// We avoid making a copy of the full incomplete batch collections
to allow
+// garbage collection.
Review comment:
We do have a reference to incomplete, however the sender will remove the
producer batches from the original incomplete collection. It's not currently
doing so because we're making a full copy of incomplete.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] chia7712 merged pull request #10604: MINOR: system test spelling/pydoc/dead code fixes
chia7712 merged pull request #10604: URL: https://github.com/apache/kafka/pull/10604 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10619: MINOR: Update test libraries and gradle plugins for better JDK 16/17 support
chia7712 commented on a change in pull request #10619:
URL: https://github.com/apache/kafka/pull/10619#discussion_r624523356
##
File path: build.gradle
##
@@ -30,15 +30,15 @@ buildscript {
}
plugins {
- id 'com.diffplug.spotless' version '5.10.2'
- id 'com.github.ben-manes.versions' version '0.36.0'
+ id 'com.diffplug.spotless' version '5.12.4'
+ id 'com.github.ben-manes.versions' version '0.38.0'
id 'idea'
id 'java-library'
id 'org.owasp.dependencycheck' version '6.1.1'
Review comment:
The latest version of `org.owasp.dependencycheck` is 6.1.6. Maybe we can
update it also?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10446: KAFKA-12661 ConfigEntry#equal does not compare other fields when value is NOT null
ijuma commented on a change in pull request #10446:
URL: https://github.com/apache/kafka/pull/10446#discussion_r624518845
##
File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala
##
@@ -803,7 +803,14 @@ class ConfigCommandTest extends ZooKeeperTestHarness with
Logging {
new AlterConfigOp(newConfigEntry("min.insync.replicas", "2"),
AlterConfigOp.OpType.SET),
new AlterConfigOp(newConfigEntry("unclean.leader.election.enable",
""), AlterConfigOp.OpType.DELETE)
)
-assertEquals(expectedConfigOps, alterConfigOps.asScala.toSet)
+assertEquals(expectedConfigOps.size, alterConfigOps.size())
+expectedConfigOps.foreach { expectedOp =>
+ val actual = alterConfigOps.asScala.find(_.configEntry().name() ==
expectedOp.configEntry().name())
Review comment:
Nit: no need for `()` in many places.
##
File path: clients/src/main/java/org/apache/kafka/clients/admin/ConfigEntry.java
##
@@ -149,24 +149,28 @@ public boolean equals(Object o) {
ConfigEntry that = (ConfigEntry) o;
-return this.name.equals(that.name) &&
-this.value != null ? this.value.equals(that.value) :
that.value == null &&
+return Objects.equals(this.name, that.name) &&
Review comment:
`name` cannot be `null` right?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10620: KAFKA-12736: KafkaProducer.flush holds onto completed ProducerBatch(s) until flush completes
ijuma commented on a change in pull request #10620:
URL: https://github.com/apache/kafka/pull/10620#discussion_r624518310
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
##
@@ -710,8 +710,11 @@ private boolean appendsInProgress() {
*/
public void awaitFlushCompletion() throws InterruptedException {
try {
-for (ProducerBatch batch : this.incomplete.copyAll())
-batch.produceFuture.await();
+// Make a copy of of the request results at the time the flush is
called.
+// We avoid making a copy of the full incomplete batch collections
to allow
+// garbage collection.
Review comment:
Hmm, we still have a reference to `incomplete` in the class, right? Are
we expecting the GC to free that while blocked in this method?
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java
##
@@ -710,8 +710,11 @@ private boolean appendsInProgress() {
*/
public void awaitFlushCompletion() throws InterruptedException {
try {
-for (ProducerBatch batch : this.incomplete.copyAll())
-batch.produceFuture.await();
+// Make a copy of of the request results at the time the flush is
called.
+// We avoid making a copy of the full incomplete batch collections
to allow
+// garbage collection.
Review comment:
Hmm, we still have a reference to `incomplete` in this instance, right?
Are we expecting the GC to free that while blocked in this method?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
