[GitHub] [kafka] C0urante commented on pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties
C0urante commented on pull request #8608: URL: https://github.com/apache/kafka/pull/8608#issuecomment-623257536 @ryannedolan based on your experience MM2, would you be willing to take a look at this? 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] C0urante opened a new pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties
C0urante opened a new pull request #8608:
URL: https://github.com/apache/kafka/pull/8608
[Jira](https://issues.apache.org/jira/browse/KAFKA-9950)
MM2 is currently sharing the same `ConfigDef` object for all its connectors
and tasks, which would be fine _if_ that object were used as-is. However, the
`MirrorTaskConfig` class mutates the `ConfigDef` by defining additional
properties, which leads to a potential `ConcurrentModificationException` during
worker configuration validation and unintended inclusion of those new
properties in the `ConfigDef` for the connectors which in turn is then visible
via the REST API's `/connectors/{name}/config/validate` endpoint.
The fix here is a one-liner that just creates a copy of the `ConfigDef`
before defining new properties.
A unit test is added that fails without this fix and passes with it.
### 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] [Created] (KAFKA-9950) MirrorMaker2 sharing of ConfigDef can lead to ConcurrentModificationException
Chris Egerton created KAFKA-9950:
Summary: MirrorMaker2 sharing of ConfigDef can lead to
ConcurrentModificationException
Key: KAFKA-9950
URL: https://issues.apache.org/jira/browse/KAFKA-9950
Project: Kafka
Issue Type: Bug
Components: mirrormaker
Affects Versions: 2.4.1, 2.5.0, 2.4.0
Reporter: Chris Egerton
Assignee: Chris Egerton
The
[MirrorConnectorConfig::CONNECTOR_CONFIG_DEF|https://github.com/apache/kafka/blob/34824b7bff64ba387a04466d74ac6bbbd10bf37c/connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorConnectorConfig.java#L397]
object is reused across multiple MirrorMaker2 classes, which is fine the most
part since it's a constant. However, the actual {{ConfigDef}} object itself is
mutable, and is mutated when the {{MirrorTaskConfig}} class [statically
constructs its own
ConfigDef|https://github.com/apache/kafka/blob/34824b7bff64ba387a04466d74ac6bbbd10bf37c/connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorTaskConfig.java#L62].
This has two unintended effects:
# Since the two {{ConfigDef}} objects for the {{MirrorConnectorConfig}} and
{{MirrorTaskConfig}} classes are actually the same object, the additional
properties that the {{MirrorTaskConfig}} class defines for its {{ConfigDef}}
are also added to the {{MirrorConnectorConfig}} class's {{ConfigDef}}. The
impact of this isn't huge since both additional properties have default values,
but this does cause those properties to appear in the
{{/connectors/\{name}/config/validate}} endpoint once the {{MirrorTaskConfig}}
class is loaded for the first time.
# It's possible that, if a config for a MirrorMaker2 connector is submitted at
approximately the same time that the {{MirrorTaskConfig}} class is loaded, a
{{ConcurrentModificationException}} will be thrown by the {{AbstractHerder}}
class when it tries to [iterate over all of the keys of the connector's
ConfigDef|https://github.com/apache/kafka/blob/34824b7bff64ba387a04466d74ac6bbbd10bf37c/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java#L357].
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] ijuma commented on pull request #8607: KAFKA-9731: Disable immediate fetch response for hw propagation if replica selector is not defined
ijuma commented on pull request #8607: URL: https://github.com/apache/kafka/pull/8607#issuecomment-623240751 1 job passed, 1 failed due to a spotBugs issue with Scala 2.12 and one failed with an unrelated flaky test failure: > org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin I pushed a fix for the spotBugs issue and will try to add a test verifying this behavior since we don't see to have any. 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] mjsax commented on pull request #8603: MINOR: Fix ProcessorContext JavaDocs
mjsax commented on pull request #8603: URL: https://github.com/apache/kafka/pull/8603#issuecomment-623240662 Java 8 and 14 passed. Java 11: `org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication` 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-9013) Flaky Test MirrorConnectorsIntegrationTest#testReplication
[
https://issues.apache.org/jira/browse/KAFKA-9013?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098641#comment-17098641
]
Matthias J. Sax commented on KAFKA-9013:
[https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/6121/testReport/junit/org.apache.kafka.connect.mirror/MirrorConnectorsIntegrationTest/testReplication/]
{quote}java.lang.AssertionError: Connector MirrorCheckpointConnector tasks did
not start in time on cluster:
org.apache.kafka.connect.util.clusters.EmbeddedConnectCluster@8715441 at
org.apache.kafka.connect.util.clusters.EmbeddedConnectClusterAssertions.assertConnectorAndAtLeastNumTasksAreRunning(EmbeddedConnectClusterAssertions.java:165)
at
org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.waitUntilMirrorMakerIsRunning(MirrorConnectorsIntegrationTest.java:191)
at
org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.setup(MirrorConnectorsIntegrationTest.java:184){quote}
> Flaky Test MirrorConnectorsIntegrationTest#testReplication
> --
>
> Key: KAFKA-9013
> URL: https://issues.apache.org/jira/browse/KAFKA-9013
> Project: Kafka
> Issue Type: Bug
> Components: KafkaConnect
>Reporter: Bruno Cadonna
>Priority: Major
> Labels: flaky-test
>
> h1. Stacktrace:
> {code:java}
> java.lang.AssertionError: Condition not met within timeout 2. Offsets not
> translated downstream to primary cluster.
> at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:377)
> at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:354)
> at
> org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication(MirrorConnectorsIntegrationTest.java:239)
> {code}
> h1. Standard Error
> {code}
> Standard Error
> Oct 09, 2019 11:32:00 PM org.glassfish.jersey.internal.inject.Providers
> checkProviderRuntime
> WARNING: A provider
> org.apache.kafka.connect.runtime.rest.resources.ConnectorPluginsResource
> registered in SERVER runtime does not implement any provider interfaces
> applicable in the SERVER runtime. Due to constraint configuration problems
> the provider
> org.apache.kafka.connect.runtime.rest.resources.ConnectorPluginsResource will
> be ignored.
> Oct 09, 2019 11:32:00 PM org.glassfish.jersey.internal.inject.Providers
> checkProviderRuntime
> WARNING: A provider
> org.apache.kafka.connect.runtime.rest.resources.LoggingResource registered in
> SERVER runtime does not implement any provider interfaces applicable in the
> SERVER runtime. Due to constraint configuration problems the provider
> org.apache.kafka.connect.runtime.rest.resources.LoggingResource will be
> ignored.
> Oct 09, 2019 11:32:00 PM org.glassfish.jersey.internal.inject.Providers
> checkProviderRuntime
> WARNING: A provider
> org.apache.kafka.connect.runtime.rest.resources.ConnectorsResource registered
> in SERVER runtime does not implement any provider interfaces applicable in
> the SERVER runtime. Due to constraint configuration problems the provider
> org.apache.kafka.connect.runtime.rest.resources.ConnectorsResource will be
> ignored.
> Oct 09, 2019 11:32:00 PM org.glassfish.jersey.internal.inject.Providers
> checkProviderRuntime
> WARNING: A provider
> org.apache.kafka.connect.runtime.rest.resources.RootResource registered in
> SERVER runtime does not implement any provider interfaces applicable in the
> SERVER runtime. Due to constraint configuration problems the provider
> org.apache.kafka.connect.runtime.rest.resources.RootResource will be ignored.
> Oct 09, 2019 11:32:01 PM org.glassfish.jersey.internal.Errors logErrors
> WARNING: The following warnings have been detected: WARNING: The
> (sub)resource method listLoggers in
> org.apache.kafka.connect.runtime.rest.resources.LoggingResource contains
> empty path annotation.
> WARNING: The (sub)resource method listConnectors in
> org.apache.kafka.connect.runtime.rest.resources.ConnectorsResource contains
> empty path annotation.
> WARNING: The (sub)resource method createConnector in
> org.apache.kafka.connect.runtime.rest.resources.ConnectorsResource contains
> empty path annotation.
> WARNING: The (sub)resource method listConnectorPlugins in
> org.apache.kafka.connect.runtime.rest.resources.ConnectorPluginsResource
> contains empty path annotation.
> WARNING: The (sub)resource method serverInfo in
> org.apache.kafka.connect.runtime.rest.resources.RootResource contains empty
> path annotation.
> Oct 09, 2019 11:32:02 PM org.glassfish.jersey.internal.inject.Providers
> checkProviderRuntime
> WARNING: A provider
> org.apache.kafka.connect.runtime.rest.resources.ConnectorPluginsResource
> registered in SERVER runtime does not implement any provider interfaces
> applicable in the SERVER runtime. Due to constraint configuration problems
[GitHub] [kafka] mjsax commented on pull request #8600: KAFKA-9928: Fix flaky GlobalKTableEOSIntegrationTest
mjsax commented on pull request #8600: URL: https://github.com/apache/kafka/pull/8600#issuecomment-623238235 Retest this please. 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] mjsax commented on pull request #8600: KAFKA-9928: Fix flaky GlobalKTableEOSIntegrationTest
mjsax commented on pull request #8600:
URL: https://github.com/apache/kafka/pull/8600#issuecomment-623237799
Java 8 passed.
Java 11:
`org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin`
(note it's not the EOS test)
Java 14:
```
org.apache.kafka.streams.integration.GlobalKTableEOSIntegrationTest.shouldKStreamGlobalKTableLeftJoin[exactly_once_beta]
org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin
```
Java 14 EOS test failed with:
```
java.lang.AssertionError: Condition not met within timeout 3. waiting
for final values
expected: {a=1+F, b=2+G, c=3+H, d=4+I, e=5+J}
received: {a=1+F, b=2+G, c=3+C, d=4+I, e=5+J}
```
It seem we are missing one update, but it's unclear why/how an input record
could get dropped... Will investigate further.
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-9949) Flaky Test GlobalKTableIntegrationTest#shouldKStreamGlobalKTableLeftJoin
[
https://issues.apache.org/jira/browse/KAFKA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098636#comment-17098636
]
Matthias J. Sax edited comment on KAFKA-9949 at 5/4/20, 3:08 AM:
-
And one more:
[https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/6117/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin/]
Failed in same run in retry, too:
[https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/6117/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin_2/]
was (Author: mjsax):
And one more:
[https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/6117/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin/]
> Flaky Test GlobalKTableIntegrationTest#shouldKStreamGlobalKTableLeftJoin
>
>
> Key: KAFKA-9949
> URL: https://issues.apache.org/jira/browse/KAFKA-9949
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Reporter: Matthias J. Sax
>Priority: Critical
> Labels: flaky-test
>
> [https://builds.apache.org/job/kafka-pr-jdk14-scala2.13/248/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin/]
> {quote}java.lang.AssertionError: Condition not met within timeout 3.
> waiting for final values at
> org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:26) at
> org.apache.kafka.test.TestUtils.lambda$waitForCondition$5(TestUtils.java:381)
> at
> org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:429)
> at
> org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:397)
> at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:378) at
> org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:368) at
> org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin(GlobalKTableIntegrationTest.java:175){quote}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (KAFKA-9949) Flaky Test GlobalKTableIntegrationTest#shouldKStreamGlobalKTableLeftJoin
Matthias J. Sax created KAFKA-9949:
--
Summary: Flaky Test
GlobalKTableIntegrationTest#shouldKStreamGlobalKTableLeftJoin
Key: KAFKA-9949
URL: https://issues.apache.org/jira/browse/KAFKA-9949
Project: Kafka
Issue Type: Improvement
Components: streams, unit tests
Reporter: Matthias J. Sax
[https://builds.apache.org/job/kafka-pr-jdk14-scala2.13/248/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin/]
{quote}java.lang.AssertionError: Condition not met within timeout 3.
waiting for final values at
org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:26) at
org.apache.kafka.test.TestUtils.lambda$waitForCondition$5(TestUtils.java:381)
at
org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:429)
at
org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:397)
at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:378) at
org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:368) at
org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin(GlobalKTableIntegrationTest.java:175){quote}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (KAFKA-9949) Flaky Test GlobalKTableIntegrationTest#shouldKStreamGlobalKTableLeftJoin
[
https://issues.apache.org/jira/browse/KAFKA-9949?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098636#comment-17098636
]
Matthias J. Sax commented on KAFKA-9949:
And one more:
[https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/6117/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin/]
> Flaky Test GlobalKTableIntegrationTest#shouldKStreamGlobalKTableLeftJoin
>
>
> Key: KAFKA-9949
> URL: https://issues.apache.org/jira/browse/KAFKA-9949
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Reporter: Matthias J. Sax
>Priority: Critical
> Labels: flaky-test
>
> [https://builds.apache.org/job/kafka-pr-jdk14-scala2.13/248/testReport/junit/org.apache.kafka.streams.integration/GlobalKTableIntegrationTest/shouldKStreamGlobalKTableLeftJoin/]
> {quote}java.lang.AssertionError: Condition not met within timeout 3.
> waiting for final values at
> org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:26) at
> org.apache.kafka.test.TestUtils.lambda$waitForCondition$5(TestUtils.java:381)
> at
> org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:429)
> at
> org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:397)
> at org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:378) at
> org.apache.kafka.test.TestUtils.waitForCondition(TestUtils.java:368) at
> org.apache.kafka.streams.integration.GlobalKTableIntegrationTest.shouldKStreamGlobalKTableLeftJoin(GlobalKTableIntegrationTest.java:175){quote}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] ijuma commented on a change in pull request #8605: Minor: remove KafkaProducer#propsToMap as it is duplicate to Abstract…
ijuma commented on a change in pull request #8605:
URL: https://github.com/apache/kafka/pull/8605#discussion_r419185106
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##
@@ -284,7 +283,8 @@ public KafkaProducer(final Map configs) {
* be called in the producer when the serializer
is passed in directly.
*/
public KafkaProducer(Map configs, Serializer
keySerializer, Serializer valueSerializer) {
-this(configs, keySerializer, valueSerializer, null, null, null,
Time.SYSTEM);
+this(new ProducerConfig(ProducerConfig.addSerializerToConfig(configs,
keySerializer, valueSerializer)),
Review comment:
Yeah, it's a bit error prone to have that logic in every constructor. We
could move the `propsToMap` method to a utility class and use it on the
consumer too.
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 #8605: Minor: remove KafkaProducer#propsToMap as it is duplicate to Abstract…
chia7712 commented on a change in pull request #8605:
URL: https://github.com/apache/kafka/pull/8605#discussion_r419180021
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##
@@ -284,7 +283,8 @@ public KafkaProducer(final Map configs) {
* be called in the producer when the serializer
is passed in directly.
*/
public KafkaProducer(Map configs, Serializer
keySerializer, Serializer valueSerializer) {
-this(configs, keySerializer, valueSerializer, null, null, null,
Time.SYSTEM);
+this(new ProducerConfig(ProducerConfig.addSerializerToConfig(configs,
keySerializer, valueSerializer)),
Review comment:
Thanks for reviews!
Do you mean that previous approach tried to avoid
‘’’addSerializerToConfig’’’? If so, should we do it for KafkaConsumer?
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 opened a new pull request #8607: KAFKA-9731: Disable immediate fetch response for hw propagation if replica selector is not defined
ijuma opened a new pull request #8607: URL: https://github.com/apache/kafka/pull/8607 In the case described in the JIRA, there was a 50%+ increase in the total fetch request rate in 2.4.0 due to this change. I included a few additional clean-ups: * Simplify `findPreferredReadReplica` and avoid unnecessary collection copies. * Use `LongSupplier` instead of `Supplier` in `SubscriptionState` to avoid unnecessary boxing. ### 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
[GitHub] [kafka] gwenshap opened a new pull request #8606: KAFKA-9731: No need to propagate HWM to followers when using default …
gwenshap opened a new pull request #8606: URL: https://github.com/apache/kafka/pull/8606 Attempted fix to the unnecessary fetch request responses detected by @vahidhashemian in KAFKA-9731. I did not add tests (under the assumption that the correctness of HWM propagation is already tested, and those tests will validate my change). I also didn't attempt to reproduce the issue that Vahid spotted, but rather implemented the solution he suggested. 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 #8602: KAFKA-9947; Ensure proper shutdown of components in `TransactionsBounceTest`
ijuma commented on a change in pull request #8602:
URL: https://github.com/apache/kafka/pull/8602#discussion_r419160617
##
File path:
core/src/test/scala/integration/kafka/api/TransactionsBounceTest.scala
##
@@ -130,13 +129,10 @@ class TransactionsBounceTest extends
KafkaServerTestHarness {
iteration += 1
}
} finally {
- producer.close()
- consumer.close()
Review comment:
We are now relying on the lifecycle methods to close the producer and
consumer. Did you do that to simplify the code, i.e. it has no impact on the
behavior of the test?
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 #8585: KAFKA-9938; Debug consumer should be able to fetch from followers
ijuma commented on a change in pull request #8585: URL: https://github.com/apache/kafka/pull/8585#discussion_r419159734 ## File path: core/src/main/scala/kafka/server/ReplicaManager.scala ## @@ -949,8 +949,11 @@ class ReplicaManager(val config: KafkaConfig, else FetchHighWatermark -// Restrict fetching to leader if request is from follower or from a client with older version (no ClientMetadata) -val fetchOnlyFromLeader = isFromFollower || (isFromConsumer && clientMetadata.isEmpty) +// Restrict fetching to leader if request is from follower or from an ordinary consumer +// with an older version (which is implied by no ClientMetadata) Review comment: The second question is: do we want to allow this debugging consumer id behavior at all? Is it useful in the new world? 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 #8585: KAFKA-9938; Debug consumer should be able to fetch from followers
ijuma commented on a change in pull request #8585: URL: https://github.com/apache/kafka/pull/8585#discussion_r419159612 ## File path: core/src/main/scala/kafka/server/ReplicaManager.scala ## @@ -949,8 +949,11 @@ class ReplicaManager(val config: KafkaConfig, else FetchHighWatermark -// Restrict fetching to leader if request is from follower or from a client with older version (no ClientMetadata) -val fetchOnlyFromLeader = isFromFollower || (isFromConsumer && clientMetadata.isEmpty) +// Restrict fetching to leader if request is from follower or from an ordinary consumer +// with an older version (which is implied by no ClientMetadata) Review comment: Seems like this comment is a bit redundant. The main useful info is that no ClientMetadata means older version, could we have a method name that made that clear and then the comment would not be needed? 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] gwenshap commented on pull request #8456: MINOR: Annotate KafkaAdminClientTest.testAlterClientQuotas() with @Test
gwenshap commented on pull request #8456: URL: https://github.com/apache/kafka/pull/8456#issuecomment-623181092 The results of the failed test are long gone, but this is literally just a test annotation, and 2/3 test runs passed. I'll just go ahead and merge this. 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 #8605: Minor: remove KafkaProducer#propsToMap as it is duplicate to Abstract…
ijuma commented on a change in pull request #8605:
URL: https://github.com/apache/kafka/pull/8605#discussion_r419158524
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##
@@ -284,7 +283,8 @@ public KafkaProducer(final Map configs) {
* be called in the producer when the serializer
is passed in directly.
*/
public KafkaProducer(Map configs, Serializer
keySerializer, Serializer valueSerializer) {
-this(configs, keySerializer, valueSerializer, null, null, null,
Time.SYSTEM);
+this(new ProducerConfig(ProducerConfig.addSerializerToConfig(configs,
keySerializer, valueSerializer)),
Review comment:
The previous approach was intended to avoid having this logic in many
places.
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] [Assigned] (KAFKA-9731) Increased fetch request rate with leader selector due to HW propagation
[ https://issues.apache.org/jira/browse/KAFKA-9731?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ismael Juma reassigned KAFKA-9731: -- Assignee: Ismael Juma > Increased fetch request rate with leader selector due to HW propagation > --- > > Key: KAFKA-9731 > URL: https://issues.apache.org/jira/browse/KAFKA-9731 > Project: Kafka > Issue Type: Improvement > Components: core >Affects Versions: 2.4.0, 2.4.1 >Reporter: Vahid Hashemian >Assignee: Ismael Juma >Priority: Major > Attachments: image-2020-03-17-10-19-08-987.png > > > KIP-392 adds high watermark propagation to followers as a means to better > sync up followers HW with leader. The issue we have noticed after trying out > 2.4.0 and 2.4.1 is a spike in fetch request rate in the default selector case > (leader), that does not really require this high watermark propagation: > !image-2020-03-17-10-19-08-987.png|width=811,height=354! > This spike causes an increase in resource allocation (CPU) on the brokers. > An easy solution would be to disable this propagation (at least) for the > default leader selector case to improve the backward compatibility. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] chia7712 commented on a change in pull request #8605: Minor: remove KafkaProducer#propsToMap as it is duplicate to Abstract…
chia7712 commented on a change in pull request #8605:
URL: https://github.com/apache/kafka/pull/8605#discussion_r419105803
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##
@@ -310,21 +310,31 @@ public KafkaProducer(Properties properties) {
* be called in the producer when the serializer
is passed in directly.
*/
public KafkaProducer(Properties properties, Serializer keySerializer,
Serializer valueSerializer) {
-this(propsToMap(properties), keySerializer, valueSerializer, null,
null, null,
+this(new
ProducerConfig(ProducerConfig.addSerializerToConfig(properties, keySerializer,
valueSerializer)), keySerializer,
+valueSerializer, null, null, null,
Time.SYSTEM);
}
// visible for testing
-@SuppressWarnings("unchecked")
KafkaProducer(Map configs,
Review comment:
this constructor is used by KafkaProducerTest only. I keep this method
to minimize this PR
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 #8605: Minor: remove KafkaProducer#propsToMap as it is duplicate to Abstract…
chia7712 commented on a change in pull request #8605:
URL: https://github.com/apache/kafka/pull/8605#discussion_r419105581
##
File path:
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
##
@@ -1235,19 +1245,6 @@ private void close(Duration timeout, boolean
swallowException) {
log.debug("Kafka producer has been closed");
}
-private static Map propsToMap(Properties properties) {
Review comment:
this check is duplicate to
https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java#L103
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 opened a new pull request #8605: Minor: remove KafkaProducer#propsToMap as it is duplicate to Abstract…
chia7712 opened a new pull request #8605: URL: https://github.com/apache/kafka/pull/8605 This PR includes following changes. 1. remove KafkaProducer#propsToMap 1. align the constructor of KafkaProducer and KafkaConsumer ### 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
[GitHub] [kafka] mimaison opened a new pull request #8604: KIP-597: MirrorMaker2 internal topics Formatters
mimaison opened a new pull request #8604: URL: https://github.com/apache/kafka/pull/8604 This PR includes 3 MessageFormatters for MirrorMaker2 internal topics: - HeartbeatFormatter - CheckpointFormatter - OffsetSyncFormatter This also introduces a new public interface org.apache.kafka.common.MessageFormatter that users can implement to build custom formatters. ### 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-8733) Offline partitions occur when leader's disk is slow in reads while responding to follower fetch requests.
[
https://issues.apache.org/jira/browse/KAFKA-8733?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098264#comment-17098264
]
Ming Liu commented on KAFKA-8733:
-
We (at Twitter) also saw this issue almost every month and it is annoying.
Given it is availability loss, we have to react very fast and set unclean
leader election.
Given when this happens, we should also starts the disk/host swap operation. It
seems the low level system metrics or fetch latency metrics are good metrics to
monitor such scenario. We should add a metrics monitor such exact scenario?
Also another observation, given we are using min-ISR=2. So the last replica
kicked out of ISR should have the same HW as the eventual offline leader. So
when you set the unclean leader election, there is 50% chance you incur a data
loss(if the election doesn't select that replica).
> Offline partitions occur when leader's disk is slow in reads while responding
> to follower fetch requests.
> -
>
> Key: KAFKA-8733
> URL: https://issues.apache.org/jira/browse/KAFKA-8733
> Project: Kafka
> Issue Type: Bug
> Components: core
>Affects Versions: 1.1.2, 2.4.0
>Reporter: Satish Duggana
>Assignee: Satish Duggana
>Priority: Critical
> Attachments: weighted-io-time-2.png, wio-time.png
>
>
> We found offline partitions issue multiple times on some of the hosts in our
> clusters. After going through the broker logs and hosts’s disk stats, it
> looks like this issue occurs whenever the read/write operations take more
> time on that disk. In a particular case where read time is more than the
> replica.lag.time.max.ms, follower replicas will be out of sync as their
> earlier fetch requests are stuck while reading the local log and their fetch
> status is not yet updated as mentioned in the below code of `ReplicaManager`.
> If there is an issue in reading the data from the log for a duration more
> than replica.lag.time.max.ms then all the replicas will be out of sync and
> partition becomes offline if min.isr.replicas > 1 and unclean.leader.election
> is false.
>
> {code:java}
> def readFromLog(): Seq[(TopicPartition, LogReadResult)] = {
> val result = readFromLocalLog( // this call took more than
> `replica.lag.time.max.ms`
> replicaId = replicaId,
> fetchOnlyFromLeader = fetchOnlyFromLeader,
> readOnlyCommitted = fetchOnlyCommitted,
> fetchMaxBytes = fetchMaxBytes,
> hardMaxBytesLimit = hardMaxBytesLimit,
> readPartitionInfo = fetchInfos,
> quota = quota,
> isolationLevel = isolationLevel)
> if (isFromFollower) updateFollowerLogReadResults(replicaId, result). //
> fetch time gets updated here, but mayBeShrinkIsr should have been already
> called and the replica is removed from isr
> else result
> }
> val logReadResults = readFromLog()
> {code}
> Attached the graphs of disk weighted io time stats when this issue occurred.
> I will raise [KIP-501|https://s.apache.org/jhbpn] describing options on how
> to handle this scenario.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
