[GitHub] [kafka] abbccdda opened a new pull request #10809: MINOR: Style fixes to KafkaRaftClient
abbccdda opened a new pull request #10809: URL: https://github.com/apache/kafka/pull/10809 As title suggested, made some style fixes to the class `KafkaRaftClient` ### 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] mjsax commented on pull request #10668: MINOR: Apply try-with-resource to KafkaStreamsTest
mjsax commented on pull request #10668: URL: https://github.com/apache/kafka/pull/10668#issuecomment-853547323 Thanks @vitojeng! Merged 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] mjsax merged pull request #10668: MINOR: Apply try-with-resource to KafkaStreamsTest
mjsax merged pull request #10668: URL: https://github.com/apache/kafka/pull/10668 -- 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-12629) Failing Test: RaftClusterTest
[
https://issues.apache.org/jira/browse/KAFKA-12629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356149#comment-17356149
]
Matthias J. Sax commented on KAFKA-12629:
-
Sweet!
Failed again:
[https://github.com/apache/kafka/pull/10668/checks?check_run_id=2732992559]
> Failing Test: RaftClusterTest
> -
>
> Key: KAFKA-12629
> URL: https://issues.apache.org/jira/browse/KAFKA-12629
> Project: Kafka
> Issue Type: Test
> Components: core, unit tests
>Reporter: Matthias J. Sax
>Assignee: Luke Chen
>Priority: Blocker
> Labels: flaky-test
> Fix For: 3.0.0
>
>
> {quote} {{java.util.concurrent.ExecutionException:
> java.lang.ClassNotFoundException:
> org.apache.kafka.controller.NoOpSnapshotWriterBuilder
> at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
> at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
> at
> kafka.testkit.KafkaClusterTestKit.startup(KafkaClusterTestKit.java:364)
> at
> kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopicsWithManyPartitions(RaftClusterTest.scala:181)}}{quote}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] saddays commented on pull request #10781: MINOR: Reduce duplicate authentication check
saddays commented on pull request #10781: URL: https://github.com/apache/kafka/pull/10781#issuecomment-853546556 > I don't think this is correct, since we are verifying the node authentication status after request being disconnected. Do you recall the reasoning for calling this API like this? @hachikuji Thanks for your review. The node authentication status is updated by **NetworkClient.processDisconnection** , so the node authentication status is **known** after disconnecting.**NetworkClient.authenticationException** just read the status of the node, not realy connect it. -- 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] [Updated] (KAFKA-12883) Adress KIP-100 type constraints now that Java 7 support is dropped
[
https://issues.apache.org/jira/browse/KAFKA-12883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
A. Sophie Blee-Goldman updated KAFKA-12883:
---
Labels: StarterProject newbie++ (was: StarterProject streams)
> Adress KIP-100 type constraints now that Java 7 support is dropped
> --
>
> Key: KAFKA-12883
> URL: https://issues.apache.org/jira/browse/KAFKA-12883
> Project: Kafka
> Issue Type: Improvement
> Components: streams
>Reporter: Xavier Léauté
>Priority: Minor
> Labels: StarterProject, newbie++
>
> As part of [KIP-100 rejected
> alternatives|https://cwiki.apache.org/confluence/display/KAFKA/KIP-100+-+Relax+Type+constraints+in+Kafka+Streams+API#KIP100RelaxTypeconstraintsinKafkaStreamsAPI-RejectedAlternatives],
> we suggested a more correct alternative to the type constraints for some of
> the {{KStream}} methods.
> Unfortunately at the time, there was a Java 7 compiler behavior that
> prevented us from using those type constraints, so we had to relax them in
> order to preserve backwards compatibility.
> As part of the KIP it was mentioned that:
> ??Once we drop support for 1.7 we can always decide to switch to approach 2.
> without breaking source compatibility, by making a proposal similar to this
> KIP.??
> Since Java 7 support has been dropped a while ago, it would be a good time to
> revisit this and possibly switch to the alternative type constraints. The
> change should be source compatible, although the streams APIs have
> significantly evolved since, so there might be some additional investigation
> required to ensure that is still the case and also covers the Scala Streams
> APIs.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (KAFKA-12883) Adress KIP-100 type constraints now that Java 7 support is dropped
[
https://issues.apache.org/jira/browse/KAFKA-12883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
A. Sophie Blee-Goldman updated KAFKA-12883:
---
Component/s: streams
> Adress KIP-100 type constraints now that Java 7 support is dropped
> --
>
> Key: KAFKA-12883
> URL: https://issues.apache.org/jira/browse/KAFKA-12883
> Project: Kafka
> Issue Type: Improvement
> Components: streams
>Reporter: Xavier Léauté
>Priority: Minor
> Labels: StarterProject, streams
>
> As part of [KIP-100 rejected
> alternatives|https://cwiki.apache.org/confluence/display/KAFKA/KIP-100+-+Relax+Type+constraints+in+Kafka+Streams+API#KIP100RelaxTypeconstraintsinKafkaStreamsAPI-RejectedAlternatives],
> we suggested a more correct alternative to the type constraints for some of
> the {{KStream}} methods.
> Unfortunately at the time, there was a Java 7 compiler behavior that
> prevented us from using those type constraints, so we had to relax them in
> order to preserve backwards compatibility.
> As part of the KIP it was mentioned that:
> ??Once we drop support for 1.7 we can always decide to switch to approach 2.
> without breaking source compatibility, by making a proposal similar to this
> KIP.??
> Since Java 7 support has been dropped a while ago, it would be a good time to
> revisit this and possibly switch to the alternative type constraints. The
> change should be source compatible, although the streams APIs have
> significantly evolved since, so there might be some additional investigation
> required to ensure that is still the case and also covers the Scala Streams
> APIs.
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Created] (KAFKA-12883) Adress KIP-100 type constraints now that Java 7 support is dropped
Xavier Léauté created KAFKA-12883:
-
Summary: Adress KIP-100 type constraints now that Java 7 support
is dropped
Key: KAFKA-12883
URL: https://issues.apache.org/jira/browse/KAFKA-12883
Project: Kafka
Issue Type: Improvement
Reporter: Xavier Léauté
As part of [KIP-100 rejected
alternatives|https://cwiki.apache.org/confluence/display/KAFKA/KIP-100+-+Relax+Type+constraints+in+Kafka+Streams+API#KIP100RelaxTypeconstraintsinKafkaStreamsAPI-RejectedAlternatives],
we suggested a more correct alternative to the type constraints for some of
the {{KStream}} methods.
Unfortunately at the time, there was a Java 7 compiler behavior that prevented
us from using those type constraints, so we had to relax them in order to
preserve backwards compatibility.
As part of the KIP it was mentioned that:
??Once we drop support for 1.7 we can always decide to switch to approach 2.
without breaking source compatibility, by making a proposal similar to this
KIP.??
Since Java 7 support has been dropped a while ago, it would be a good time to
revisit this and possibly switch to the alternative type constraints. The
change should be source compatible, although the streams APIs have
significantly evolved since, so there might be some additional investigation
required to ensure that is still the case and also covers the Scala Streams
APIs.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] hachikuji commented on pull request #10793: KAFKA-12338: Remove useless MetadataParser
hachikuji commented on pull request #10793: URL: https://github.com/apache/kafka/pull/10793#issuecomment-853495182 @dengziming Thanks for the patch. Looks like the parser has some additional validations which are not present in `MetadataRecordSerde`. Are any of them worth porting over? -- 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-9220) TimeoutException when using kafka-preferred-replica-election
[ https://issues.apache.org/jira/browse/KAFKA-9220?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] loboxu reassigned KAFKA-9220: - Assignee: loboxu > TimeoutException when using kafka-preferred-replica-election > > > Key: KAFKA-9220 > URL: https://issues.apache.org/jira/browse/KAFKA-9220 > Project: Kafka > Issue Type: Bug > Components: admin >Affects Versions: 2.3.0 >Reporter: Or Shemesh >Assignee: loboxu >Priority: Major > > When running kafka-preferred-replica-election --bootstrap-server xxx:9092 > I'm getting this error: > Timeout waiting for election resultsTimeout waiting for election > resultsException in thread "main" kafka.common.AdminCommandFailedException at > kafka.admin.PreferredReplicaLeaderElectionCommand$AdminClientCommand.electPreferredLeaders(PreferredReplicaLeaderElectionCommand.scala:246) > at > kafka.admin.PreferredReplicaLeaderElectionCommand$.run(PreferredReplicaLeaderElectionCommand.scala:78) > at > kafka.admin.PreferredReplicaLeaderElectionCommand$.main(PreferredReplicaLeaderElectionCommand.scala:42) > at > kafka.admin.PreferredReplicaLeaderElectionCommand.main(PreferredReplicaLeaderElectionCommand.scala)Caused > by: org.apache.kafka.common.errors.TimeoutException: Aborted due to timeout. > > Because we have a big cluster and getting all the data from the zookeeper is > taking more the 30 second. > > After searching the code I saw that the 30 second is hard-coded can you > enable us to set the timeout as parameter? > [https://github.com/confluentinc/kafka/blob/master/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala] > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-9220) TimeoutException when using kafka-preferred-replica-election
[ https://issues.apache.org/jira/browse/KAFKA-9220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356061#comment-17356061 ] loboxu commented on KAFKA-9220: --- [~akumar] Have you started work yet? If not, I can pick this up. > TimeoutException when using kafka-preferred-replica-election > > > Key: KAFKA-9220 > URL: https://issues.apache.org/jira/browse/KAFKA-9220 > Project: Kafka > Issue Type: Bug > Components: admin >Affects Versions: 2.3.0 >Reporter: Or Shemesh >Priority: Major > > When running kafka-preferred-replica-election --bootstrap-server xxx:9092 > I'm getting this error: > Timeout waiting for election resultsTimeout waiting for election > resultsException in thread "main" kafka.common.AdminCommandFailedException at > kafka.admin.PreferredReplicaLeaderElectionCommand$AdminClientCommand.electPreferredLeaders(PreferredReplicaLeaderElectionCommand.scala:246) > at > kafka.admin.PreferredReplicaLeaderElectionCommand$.run(PreferredReplicaLeaderElectionCommand.scala:78) > at > kafka.admin.PreferredReplicaLeaderElectionCommand$.main(PreferredReplicaLeaderElectionCommand.scala:42) > at > kafka.admin.PreferredReplicaLeaderElectionCommand.main(PreferredReplicaLeaderElectionCommand.scala)Caused > by: org.apache.kafka.common.errors.TimeoutException: Aborted due to timeout. > > Because we have a big cluster and getting all the data from the zookeeper is > taking more the 30 second. > > After searching the code I saw that the 30 second is hard-coded can you > enable us to set the timeout as parameter? > [https://github.com/confluentinc/kafka/blob/master/core/src/main/scala/kafka/admin/PreferredReplicaLeaderElectionCommand.scala] > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-10493) KTable out-of-order updates are not being ignored
[ https://issues.apache.org/jira/browse/KAFKA-10493?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matthias J. Sax updated KAFKA-10493: Fix Version/s: (was: 3.0.0) 4.0.0 > KTable out-of-order updates are not being ignored > - > > Key: KAFKA-10493 > URL: https://issues.apache.org/jira/browse/KAFKA-10493 > Project: Kafka > Issue Type: Bug > Components: streams >Affects Versions: 2.6.0 >Reporter: Pedro Gontijo >Assignee: Matthias J. Sax >Priority: Blocker > Fix For: 4.0.0 > > Attachments: KTableOutOfOrderBug.java > > > On a materialized KTable, out-of-order records for a given key (records which > timestamp are older than the current value in store) are not being ignored > but used to update the local store value and also being forwarded. > I believe the bug is here: > [https://github.com/apache/kafka/blob/2.6.0/streams/src/main/java/org/apache/kafka/streams/state/internals/ValueAndTimestampSerializer.java#L77] > It should return true, not false (see javadoc) > The bug impacts here: > [https://github.com/apache/kafka/blob/2.6.0/streams/src/main/java/org/apache/kafka/streams/kstream/internals/KTableSource.java#L142-L148] > I have attached a simple stream app that shows the issue happening. > Thank you! -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-10493) KTable out-of-order updates are not being ignored
[ https://issues.apache.org/jira/browse/KAFKA-10493?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356055#comment-17356055 ] Matthias J. Sax commented on KAFKA-10493: - Seems we cannot agree on a solution for now. Pushing this out for 4.0.0 release. > KTable out-of-order updates are not being ignored > - > > Key: KAFKA-10493 > URL: https://issues.apache.org/jira/browse/KAFKA-10493 > Project: Kafka > Issue Type: Bug > Components: streams >Affects Versions: 2.6.0 >Reporter: Pedro Gontijo >Assignee: Matthias J. Sax >Priority: Blocker > Fix For: 4.0.0 > > Attachments: KTableOutOfOrderBug.java > > > On a materialized KTable, out-of-order records for a given key (records which > timestamp are older than the current value in store) are not being ignored > but used to update the local store value and also being forwarded. > I believe the bug is here: > [https://github.com/apache/kafka/blob/2.6.0/streams/src/main/java/org/apache/kafka/streams/state/internals/ValueAndTimestampSerializer.java#L77] > It should return true, not false (see javadoc) > The bug impacts here: > [https://github.com/apache/kafka/blob/2.6.0/streams/src/main/java/org/apache/kafka/streams/kstream/internals/KTableSource.java#L142-L148] > I have attached a simple stream app that shows the issue happening. > Thank you! -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] socutes commented on pull request #10749: KAFKA-12773: Use UncheckedIOException when wrapping IOException
socutes commented on pull request #10749: URL: https://github.com/apache/kafka/pull/10749#issuecomment-853456306 @jsancio Code format fix completed! Thank you very much for your Review. -- 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-12317) Relax non-null key requirement for left/outer KStream joins
[ https://issues.apache.org/jira/browse/KAFKA-12317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Juan C. Gonzalez-Zurita reassigned KAFKA-12317: --- Assignee: Juan C. Gonzalez-Zurita > Relax non-null key requirement for left/outer KStream joins > --- > > Key: KAFKA-12317 > URL: https://issues.apache.org/jira/browse/KAFKA-12317 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Matthias J. Sax >Assignee: Juan C. Gonzalez-Zurita >Priority: Major > > Currently, for a stream-streams and stream-table/globalTable join > KafkaStreams drops all stream records with a `null`-key (`null`-join-key for > stream-globalTable), because for a `null`-(join)key the join is undefined: > ie, we don't have an attribute the do the table lookup (we consider the > stream-record as malformed). Note, that we define the semantics of > _left/outer_ join as: keep the stream record if no matching join record was > found. > We could relax the definition of _left_ stream-table/globalTable and > _left/outer_ stream-stream join though, and not drop `null`-(join)key stream > records, and call the ValueJoiner with a `null` "other-side" value instead: > if the stream record key (or join-key) is `null`, we could treat is as > "failed lookup" instead of treating the stream record as corrupted. > If we make this change, users that want to keep the current behavior, can add > a `filter()` before the join to drop `null`-(join)key records from the stream > explicitly. > Note that this change also requires to change the behavior if we insert a > repartition topic before the join: currently, we drop `null`-key record > before writing into the repartition topic (as we know they would be dropped > later anyway). We need to relax this behavior for a left stream-table and > left/outer stream-stream join. User need to be aware (ie, we might need to > put this into the docs and JavaDocs), that records with `null`-key would be > partitioned randomly. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-12718) SessionWindows are closed too early
[
https://issues.apache.org/jira/browse/KAFKA-12718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356044#comment-17356044
]
Juan C. Gonzalez-Zurita edited comment on KAFKA-12718 at 6/2/21, 11:34 PM:
---
I will try to get this PR tonight or tomorrow night depending on circumstances.
This test case is the last I need to update in order to agree with the gap
changes within the streams test. As to the new ticket itself I would love to
pick it up but I've not investigated it with enough depth to be certain whether
or not I could beat the 14th deadline for it. I would be happy to try, though
:) [~mjsax].
Edit: I thought it was June 14th but it's really July. Yes I'll add myself rn
was (Author: gonzur):
I will try to get this PR tonight or tomorrow night depending on circumstances.
This test case is the last I need to update in order to agree with the gap
changes within the streams test. As to the new ticket itself I would love to
pick it up but I've not investigated it with enough depth to be certain whether
or not I could beat the 14th deadline for it. I would be happy to try, though
:) [~mjsax].
> SessionWindows are closed too early
> ---
>
> Key: KAFKA-12718
> URL: https://issues.apache.org/jira/browse/KAFKA-12718
> Project: Kafka
> Issue Type: Bug
> Components: streams
>Reporter: Matthias J. Sax
>Assignee: Juan C. Gonzalez-Zurita
>Priority: Critical
> Labels: beginner, easy-fix, newbie
> Fix For: 3.0.0
>
>
> SessionWindows are defined based on a {{gap}} parameter, and also support an
> additional {{grace-period}} configuration to handle out-of-order data.
> To incorporate the session-gap a session window should only be closed at
> {{window-end + gap}} and to incorporate grace-period, the close time should
> be pushed out further to {{window-end + gap + grace}}.
> However, atm we compute the window close time as {{window-end + grace}}
> omitting the {{gap}} parameter.
> Because default grace-period is 24h most users might not notice this issues.
> Even if they set a grace period explicitly (eg, when using suppress()), they
> would most likely set a grace-period larger than gap-time not hitting the
> issue (or maybe only realize it when inspecting the behavior closely).
> However, if a user wants to disable the grace-period and sets it to zero (on
> any other value smaller than gap-time), sessions might be close too early and
> user might notice.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (KAFKA-12718) SessionWindows are closed too early
[
https://issues.apache.org/jira/browse/KAFKA-12718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356044#comment-17356044
]
Juan C. Gonzalez-Zurita commented on KAFKA-12718:
-
I will try to get this PR tonight or tomorrow night depending on circumstances.
This test case is the last I need to update in order to agree with the gap
changes within the streams test. As to the new ticket itself I would love to
pick it up but I've not investigated it with enough depth to be certain whether
or not I could beat the 14th deadline for it. I would be happy to try, though
:) [~mjsax].
> SessionWindows are closed too early
> ---
>
> Key: KAFKA-12718
> URL: https://issues.apache.org/jira/browse/KAFKA-12718
> Project: Kafka
> Issue Type: Bug
> Components: streams
>Reporter: Matthias J. Sax
>Assignee: Juan C. Gonzalez-Zurita
>Priority: Critical
> Labels: beginner, easy-fix, newbie
> Fix For: 3.0.0
>
>
> SessionWindows are defined based on a {{gap}} parameter, and also support an
> additional {{grace-period}} configuration to handle out-of-order data.
> To incorporate the session-gap a session window should only be closed at
> {{window-end + gap}} and to incorporate grace-period, the close time should
> be pushed out further to {{window-end + gap + grace}}.
> However, atm we compute the window close time as {{window-end + grace}}
> omitting the {{gap}} parameter.
> Because default grace-period is 24h most users might not notice this issues.
> Even if they set a grace period explicitly (eg, when using suppress()), they
> would most likely set a grace-period larger than gap-time not hitting the
> issue (or maybe only realize it when inspecting the behavior closely).
> However, if a user wants to disable the grace-period and sets it to zero (on
> any other value smaller than gap-time), sessions might be close too early and
> user might notice.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] vitojeng commented on pull request #10668: MINOR: Apply try-with-resource to KafkaStreamsTest
vitojeng commented on pull request #10668: URL: https://github.com/apache/kafka/pull/10668#issuecomment-853445357 Thanks @mjsax ! Please take a look. -- 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] cmccabe merged pull request #10787: KAFKA-12864: Move KafkaEventQueue into server-common
cmccabe merged pull request #10787: URL: https://github.com/apache/kafka/pull/10787 -- 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-12882) Add RegisteredBrokerCount and UnfencedBrokerCount metrics to the QuorumController
Ryan Dielhenn created KAFKA-12882: - Summary: Add RegisteredBrokerCount and UnfencedBrokerCount metrics to the QuorumController Key: KAFKA-12882 URL: https://issues.apache.org/jira/browse/KAFKA-12882 Project: Kafka Issue Type: Improvement Reporter: Ryan Dielhenn Assignee: Ryan Dielhenn Adding RegisteredBrokerCount and UnfencedBrokerCount metrics to the QuorumController. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-12718) SessionWindows are closed too early
[
https://issues.apache.org/jira/browse/KAFKA-12718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356005#comment-17356005
]
Matthias J. Sax commented on KAFKA-12718:
-
Thanks [~guozhang] – given the last message from [~gonzur], I expect that we
get a PR for it soon.
Btw: I also think that getting
https://issues.apache.org/jira/browse/KAFKA-12317 into 3.0 might be worth it?
Thoughts [~guozhang]? [~gonzur] would be be interested to pick this ticket up,
too?
> SessionWindows are closed too early
> ---
>
> Key: KAFKA-12718
> URL: https://issues.apache.org/jira/browse/KAFKA-12718
> Project: Kafka
> Issue Type: Bug
> Components: streams
>Reporter: Matthias J. Sax
>Assignee: Juan C. Gonzalez-Zurita
>Priority: Critical
> Labels: beginner, easy-fix, newbie
> Fix For: 3.0.0
>
>
> SessionWindows are defined based on a {{gap}} parameter, and also support an
> additional {{grace-period}} configuration to handle out-of-order data.
> To incorporate the session-gap a session window should only be closed at
> {{window-end + gap}} and to incorporate grace-period, the close time should
> be pushed out further to {{window-end + gap + grace}}.
> However, atm we compute the window close time as {{window-end + grace}}
> omitting the {{gap}} parameter.
> Because default grace-period is 24h most users might not notice this issues.
> Even if they set a grace period explicitly (eg, when using suppress()), they
> would most likely set a grace-period larger than gap-time not hitting the
> issue (or maybe only realize it when inspecting the behavior closely).
> However, if a user wants to disable the grace-period and sets it to zero (on
> any other value smaller than gap-time), sessions might be close too early and
> user might notice.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (KAFKA-12317) Relax non-null key requirement for left/outer KStream joins
[ https://issues.apache.org/jira/browse/KAFKA-12317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matthias J. Sax updated KAFKA-12317: Description: Currently, for a stream-streams and stream-table/globalTable join KafkaStreams drops all stream records with a `null`-key (`null`-join-key for stream-globalTable), because for a `null`-(join)key the join is undefined: ie, we don't have an attribute the do the table lookup (we consider the stream-record as malformed). Note, that we define the semantics of _left/outer_ join as: keep the stream record if no matching join record was found. We could relax the definition of _left_ stream-table/globalTable and _left/outer_ stream-stream join though, and not drop `null`-(join)key stream records, and call the ValueJoiner with a `null` "other-side" value instead: if the stream record key (or join-key) is `null`, we could treat is as "failed lookup" instead of treating the stream record as corrupted. If we make this change, users that want to keep the current behavior, can add a `filter()` before the join to drop `null`-(join)key records from the stream explicitly. Note that this change also requires to change the behavior if we insert a repartition topic before the join: currently, we drop `null`-key record before writing into the repartition topic (as we know they would be dropped later anyway). We need to relax this behavior for a left stream-table and left/outer stream-stream join. User need to be aware (ie, we might need to put this into the docs and JavaDocs), that records with `null`-key would be partitioned randomly. was: Currently, for a stream-streams and stream-table/globalTable join KafkaStreams drops all stream records with a null-key, because for a null-key the join is undefined: ie, we don't have an attribute the do the table lookup (we consider the stream-record as malformed). Note, that we define the semantics of _left_ join as: keep the stream record if no KTable record was found. We could relax the definition of _left_ join though, and not drop non-key stream records, and call the ValueJoiner with a `null` table record instead: if the stream record key is `null`, we could treat is as "failed table lookup" instead of treating the stream record as corrupted. If we make this change, users that want to keep the current behavior, can add a `filter()` before the join to drop `null`-key records from the stream explicitly. Note that this change also requires to change the behavior if we insert a repartition topic before the join: currently, we drop `null`-key record before writing into the repartition topic (as we know they would be dropped later anyway). We need to relax this behavior for a left/outer stream-table (and maybe left/outer > Relax non-null key requirement for left/outer KStream joins > --- > > Key: KAFKA-12317 > URL: https://issues.apache.org/jira/browse/KAFKA-12317 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Matthias J. Sax >Priority: Major > > Currently, for a stream-streams and stream-table/globalTable join > KafkaStreams drops all stream records with a `null`-key (`null`-join-key for > stream-globalTable), because for a `null`-(join)key the join is undefined: > ie, we don't have an attribute the do the table lookup (we consider the > stream-record as malformed). Note, that we define the semantics of > _left/outer_ join as: keep the stream record if no matching join record was > found. > We could relax the definition of _left_ stream-table/globalTable and > _left/outer_ stream-stream join though, and not drop `null`-(join)key stream > records, and call the ValueJoiner with a `null` "other-side" value instead: > if the stream record key (or join-key) is `null`, we could treat is as > "failed lookup" instead of treating the stream record as corrupted. > If we make this change, users that want to keep the current behavior, can add > a `filter()` before the join to drop `null`-(join)key records from the stream > explicitly. > Note that this change also requires to change the behavior if we insert a > repartition topic before the join: currently, we drop `null`-key record > before writing into the repartition topic (as we know they would be dropped > later anyway). We need to relax this behavior for a left stream-table and > left/outer stream-stream join. User need to be aware (ie, we might need to > put this into the docs and JavaDocs), that records with `null`-key would be > partitioned randomly. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12317) Relax non-null key requirement for left KStream joins
[ https://issues.apache.org/jira/browse/KAFKA-12317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matthias J. Sax updated KAFKA-12317: Description: Currently, for a stream-streams and stream-table/globalTable join KafkaStreams drops all stream records with a null-key, because for a null-key the join is undefined: ie, we don't have an attribute the do the table lookup (we consider the stream-record as malformed). Note, that we define the semantics of _left_ join as: keep the stream record if no KTable record was found. We could relax the definition of _left_ join though, and not drop non-key stream records, and call the ValueJoiner with a `null` table record instead: if the stream record key is `null`, we could treat is as "failed table lookup" instead of treating the stream record as corrupted. If we make this change, users that want to keep the current behavior, can add a `filter()` before the join to drop `null`-key records from the stream explicitly. Note that this change also requires to change the behavior if we insert a repartition topic before the join: currently, we drop `null`-key record before writing into the repartition topic (as we know they would be dropped later anyway). We need to relax this behavior for a left/outer stream-table (and maybe left/outer was: Currently, for a stream-table join KafkaStreams drops all stream records with a null-key, because for a null-key the join is undefined: ie, we don't have an attribute the do the table lookup (we consider the stream-record as malformed). Note, that we define the semantics of _left_ join as: keep the stream record if no KTable record was found. We could relax the definition of _left_ join though, and not drop non-key stream records, and call the ValueJoiner with a `null` table record instead: if the stream record key is `null`, we could treat is as "failed table lookup" instead of treating the stream record as corrupted. If we make this change, users that want to keep the current behavior, can add a `filter()` before the join to drop `null`-key records from the stream explicitly. Note that this change also requires to change the behavior if we insert a repartition topic before the join: currently, we drop `null`-key record before writing into the repartition topic (as we know they would be dropped later anyway). We need to relax this behavior for a left/outer stream-table (and maybe left/outer > Relax non-null key requirement for left KStream joins > - > > Key: KAFKA-12317 > URL: https://issues.apache.org/jira/browse/KAFKA-12317 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Matthias J. Sax >Priority: Major > > Currently, for a stream-streams and stream-table/globalTable join > KafkaStreams drops all stream records with a null-key, because for a null-key > the join is undefined: ie, we don't have an attribute the do the table lookup > (we consider the stream-record as malformed). Note, that we define the > semantics of _left_ join as: keep the stream record if no KTable record was > found. > We could relax the definition of _left_ join though, and not drop non-key > stream records, and call the ValueJoiner with a `null` table record instead: > if the stream record key is `null`, we could treat is as "failed table > lookup" instead of treating the stream record as corrupted. > If we make this change, users that want to keep the current behavior, can add > a `filter()` before the join to drop `null`-key records from the stream > explicitly. > Note that this change also requires to change the behavior if we insert a > repartition topic before the join: currently, we drop `null`-key record > before writing into the repartition topic (as we know they would be dropped > later anyway). We need to relax this behavior for a left/outer stream-table > (and maybe left/outer -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12317) Relax non-null key requirement for left/outer KStream joins
[ https://issues.apache.org/jira/browse/KAFKA-12317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matthias J. Sax updated KAFKA-12317: Summary: Relax non-null key requirement for left/outer KStream joins (was: Relax non-null key requirement for left KStream joins) > Relax non-null key requirement for left/outer KStream joins > --- > > Key: KAFKA-12317 > URL: https://issues.apache.org/jira/browse/KAFKA-12317 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Matthias J. Sax >Priority: Major > > Currently, for a stream-streams and stream-table/globalTable join > KafkaStreams drops all stream records with a null-key, because for a null-key > the join is undefined: ie, we don't have an attribute the do the table lookup > (we consider the stream-record as malformed). Note, that we define the > semantics of _left_ join as: keep the stream record if no KTable record was > found. > We could relax the definition of _left_ join though, and not drop non-key > stream records, and call the ValueJoiner with a `null` table record instead: > if the stream record key is `null`, we could treat is as "failed table > lookup" instead of treating the stream record as corrupted. > If we make this change, users that want to keep the current behavior, can add > a `filter()` before the join to drop `null`-key records from the stream > explicitly. > Note that this change also requires to change the behavior if we insert a > repartition topic before the join: currently, we drop `null`-key record > before writing into the repartition topic (as we know they would be dropped > later anyway). We need to relax this behavior for a left/outer stream-table > (and maybe left/outer -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12317) Relax non-null key requirement for left KStream-KTable join
[ https://issues.apache.org/jira/browse/KAFKA-12317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matthias J. Sax updated KAFKA-12317: Description: Currently, for a stream-table join KafkaStreams drops all stream records with a null-key, because for a null-key the join is undefined: ie, we don't have an attribute the do the table lookup (we consider the stream-record as malformed). Note, that we define the semantics of _left_ join as: keep the stream record if no KTable record was found. We could relax the definition of _left_ join though, and not drop non-key stream records, and call the ValueJoiner with a `null` table record instead: if the stream record key is `null`, we could treat is as "failed table lookup" instead of treating the stream record as corrupted. If we make this change, users that want to keep the current behavior, can add a `filter()` before the join to drop `null`-key records from the stream explicitly. Note that this change also requires to change the behavior if we insert a repartition topic before the join: currently, we drop `null`-key record before writing into the repartition topic (as we know they would be dropped later anyway). We need to relax this behavior for a left/outer stream-table (and maybe left/outer was: Currently, for a stream-table join KafkaStreams drops all stream records with a null-key, because for a null-key the join is undefined: ie, we don't have an attribute the do the table lookup (we consider the stream-record as malformed). Note, that we define the semantics of _left_ join as: keep the stream record if no KTable record was found. We could relax the definition of _left_ join though, and not drop non-key stream records, and call the ValueJoiner with a `null` table record instead: if the stream record key is `null`, we could treat is as "failed table lookup" instead of treating the stream record as corrupted. If we make this change, users that want to keep the current behavior, can add a `filter()` before the join to drop `null`-key records from the stream explicitly. > Relax non-null key requirement for left KStream-KTable join > --- > > Key: KAFKA-12317 > URL: https://issues.apache.org/jira/browse/KAFKA-12317 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Matthias J. Sax >Priority: Major > > Currently, for a stream-table join KafkaStreams drops all stream records with > a null-key, because for a null-key the join is undefined: ie, we don't have > an attribute the do the table lookup (we consider the stream-record as > malformed). Note, that we define the semantics of _left_ join as: keep the > stream record if no KTable record was found. > We could relax the definition of _left_ join though, and not drop non-key > stream records, and call the ValueJoiner with a `null` table record instead: > if the stream record key is `null`, we could treat is as "failed table > lookup" instead of treating the stream record as corrupted. > If we make this change, users that want to keep the current behavior, can add > a `filter()` before the join to drop `null`-key records from the stream > explicitly. > Note that this change also requires to change the behavior if we insert a > repartition topic before the join: currently, we drop `null`-key record > before writing into the repartition topic (as we know they would be dropped > later anyway). We need to relax this behavior for a left/outer stream-table > (and maybe left/outer -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12317) Relax non-null key requirement for left KStream joins
[ https://issues.apache.org/jira/browse/KAFKA-12317?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Matthias J. Sax updated KAFKA-12317: Summary: Relax non-null key requirement for left KStream joins (was: Relax non-null key requirement for left KStream-KTable join) > Relax non-null key requirement for left KStream joins > - > > Key: KAFKA-12317 > URL: https://issues.apache.org/jira/browse/KAFKA-12317 > Project: Kafka > Issue Type: Improvement > Components: streams >Reporter: Matthias J. Sax >Priority: Major > > Currently, for a stream-table join KafkaStreams drops all stream records with > a null-key, because for a null-key the join is undefined: ie, we don't have > an attribute the do the table lookup (we consider the stream-record as > malformed). Note, that we define the semantics of _left_ join as: keep the > stream record if no KTable record was found. > We could relax the definition of _left_ join though, and not drop non-key > stream records, and call the ValueJoiner with a `null` table record instead: > if the stream record key is `null`, we could treat is as "failed table > lookup" instead of treating the stream record as corrupted. > If we make this change, users that want to keep the current behavior, can add > a `filter()` before the join to drop `null`-key records from the stream > explicitly. > Note that this change also requires to change the behavior if we insert a > repartition topic before the join: currently, we drop `null`-key record > before writing into the repartition topic (as we know they would be dropped > later anyway). We need to relax this behavior for a left/outer stream-table > (and maybe left/outer -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] dejan2609 edited a comment on pull request #10698: KAFKA-12770: introduce `checkstyleVersion` build option (for overriding CheckStyle project-defined dependency version)
dejan2609 edited a comment on pull request #10698: URL: https://github.com/apache/kafka/pull/10698#issuecomment-853408053 Just tagging @ijuma here again (to bring back this PR up to the surface). -- 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] [Updated] (KAFKA-12878) Support --bootstrap-server kafka-streams-application-reset
[ https://issues.apache.org/jira/browse/KAFKA-12878?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Guozhang Wang updated KAFKA-12878: -- Priority: Critical (was: Major) > Support --bootstrap-server kafka-streams-application-reset > -- > > Key: KAFKA-12878 > URL: https://issues.apache.org/jira/browse/KAFKA-12878 > Project: Kafka > Issue Type: Improvement > Components: tools >Reporter: Neil Buesing >Assignee: Neil Buesing >Priority: Critical > Fix For: 3.0.0 > > > kafka-streams-application-reset still uses --bootstrap-servers, align with > other tools that use --bootstrap-server. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12878) Support --bootstrap-server kafka-streams-application-reset
[ https://issues.apache.org/jira/browse/KAFKA-12878?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Guozhang Wang updated KAFKA-12878: -- Fix Version/s: 3.0.0 > Support --bootstrap-server kafka-streams-application-reset > -- > > Key: KAFKA-12878 > URL: https://issues.apache.org/jira/browse/KAFKA-12878 > Project: Kafka > Issue Type: Improvement > Components: tools >Reporter: Neil Buesing >Assignee: Neil Buesing >Priority: Major > Fix For: 3.0.0 > > > kafka-streams-application-reset still uses --bootstrap-servers, align with > other tools that use --bootstrap-server. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-12878) Support --bootstrap-server kafka-streams-application-reset
[ https://issues.apache.org/jira/browse/KAFKA-12878?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356001#comment-17356001 ] Guozhang Wang commented on KAFKA-12878: --- [~nbuesing] Could you please file a small KIP to announce this as a public API change? I'm marking it as a 3.0 task for now (note the timeline of the release is here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177046466) if you have time to help pushing it towards the finishing line by then it would be great. > Support --bootstrap-server kafka-streams-application-reset > -- > > Key: KAFKA-12878 > URL: https://issues.apache.org/jira/browse/KAFKA-12878 > Project: Kafka > Issue Type: Improvement > Components: tools >Reporter: Neil Buesing >Assignee: Neil Buesing >Priority: Major > > kafka-streams-application-reset still uses --bootstrap-servers, align with > other tools that use --bootstrap-server. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-12718) SessionWindows are closed too early
[
https://issues.apache.org/jira/browse/KAFKA-12718?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356000#comment-17356000
]
Guozhang Wang commented on KAFKA-12718:
---
Hi folks, I'm bumping up the priority of this ticket for 3.0 for now (note that
we still have plenty of time towards the code freeze:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177046466 but
still would be good to get this easy fix asap).
> SessionWindows are closed too early
> ---
>
> Key: KAFKA-12718
> URL: https://issues.apache.org/jira/browse/KAFKA-12718
> Project: Kafka
> Issue Type: Bug
> Components: streams
>Reporter: Matthias J. Sax
>Assignee: Juan C. Gonzalez-Zurita
>Priority: Critical
> Labels: beginner, easy-fix, newbie
> Fix For: 3.0.0
>
>
> SessionWindows are defined based on a {{gap}} parameter, and also support an
> additional {{grace-period}} configuration to handle out-of-order data.
> To incorporate the session-gap a session window should only be closed at
> {{window-end + gap}} and to incorporate grace-period, the close time should
> be pushed out further to {{window-end + gap + grace}}.
> However, atm we compute the window close time as {{window-end + grace}}
> omitting the {{gap}} parameter.
> Because default grace-period is 24h most users might not notice this issues.
> Even if they set a grace period explicitly (eg, when using suppress()), they
> would most likely set a grace-period larger than gap-time not hitting the
> issue (or maybe only realize it when inspecting the behavior closely).
> However, if a user wants to disable the grace-period and sets it to zero (on
> any other value smaller than gap-time), sessions might be close too early and
> user might notice.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (KAFKA-12718) SessionWindows are closed too early
[
https://issues.apache.org/jira/browse/KAFKA-12718?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Guozhang Wang updated KAFKA-12718:
--
Priority: Critical (was: Major)
> SessionWindows are closed too early
> ---
>
> Key: KAFKA-12718
> URL: https://issues.apache.org/jira/browse/KAFKA-12718
> Project: Kafka
> Issue Type: Bug
> Components: streams
>Reporter: Matthias J. Sax
>Assignee: Juan C. Gonzalez-Zurita
>Priority: Critical
> Labels: beginner, easy-fix, newbie
> Fix For: 3.0.0
>
>
> SessionWindows are defined based on a {{gap}} parameter, and also support an
> additional {{grace-period}} configuration to handle out-of-order data.
> To incorporate the session-gap a session window should only be closed at
> {{window-end + gap}} and to incorporate grace-period, the close time should
> be pushed out further to {{window-end + gap + grace}}.
> However, atm we compute the window close time as {{window-end + grace}}
> omitting the {{gap}} parameter.
> Because default grace-period is 24h most users might not notice this issues.
> Even if they set a grace period explicitly (eg, when using suppress()), they
> would most likely set a grace-period larger than gap-time not hitting the
> issue (or maybe only realize it when inspecting the behavior closely).
> However, if a user wants to disable the grace-period and sets it to zero (on
> any other value smaller than gap-time), sessions might be close too early and
> user might notice.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] dejan2609 edited a comment on pull request #10698: KAFKA-12770: introduce `checkstyleVersion` build option (for overriding CheckStyle project-defined dependency version)
dejan2609 edited a comment on pull request #10698: URL: https://github.com/apache/kafka/pull/10698#issuecomment-853408053 Just tagging @ijuma here again (to put back this PR up to the surface). -- 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 commented on a change in pull request #10798: KAFKA-9168: Adding direct byte buffer support to rocksdb state store
guozhangwang commented on a change in pull request #10798:
URL: https://github.com/apache/kafka/pull/10798#discussion_r644356441
##
File path:
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBStore.java
##
@@ -505,6 +506,14 @@ private void closeOpenIterators() {
}
}
+private ByteBuffer createDirectByteBufferAndPut(byte[] bytes) {
+ByteBuffer directBuffer = ByteBuffer.allocateDirect(bytes.length);
Review comment:
I'm not very familiar with the direct buffer usage pattern, but
currently it seems we would still try to allocate a new buffer for each put
call, whereas I "thought" the main benefits come from reusing the buffer across
multiple put calls? @vamossagar12 @ableegoldman @cadonna please correct me if
I'm wrong.
--
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 commented on pull request #10552: KAFKA-12675: improve the sticky general assignor scalability and performance
guozhangwang commented on pull request #10552: URL: https://github.com/apache/kafka/pull/10552#issuecomment-853412114 Thank you @showuon !! -- 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 #10552: KAFKA-12675: improve the sticky general assignor scalability and performance
guozhangwang merged pull request #10552: URL: https://github.com/apache/kafka/pull/10552 -- 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 commented on pull request #10552: KAFKA-12675: improve the sticky general assignor scalability and performance
guozhangwang commented on pull request #10552: URL: https://github.com/apache/kafka/pull/10552#issuecomment-853411553 The failed tests are irrelevant to this PR, I'm merging to trunk now. -- 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] dejan2609 commented on pull request #10428: KAFKA-12572: Add import ordering checkstyle rule and configure an automatic formatter
dejan2609 commented on pull request #10428: URL: https://github.com/apache/kafka/pull/10428#issuecomment-853411084 Shameless plug and related to CheckStyle: ⏩ #10698 (needs a review / approval). -- 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 commented on a change in pull request #10609: KAFKA-12648: Pt. 1 - Add NamedTopology to protocol and state directory structure
guozhangwang commented on a change in pull request #10609:
URL: https://github.com/apache/kafka/pull/10609#discussion_r644345818
##
File path:
streams/src/test/java/org/apache/kafka/streams/processor/internals/StateDirectoryTest.java
##
@@ -593,6 +575,111 @@ public void shouldLogTempDirMessage() {
}
}
+/* Named Topology Tests */
+
+@Test
+public void shouldCreateTaskDirectoriesUnderNamedTopologyDirs() throws
IOException {
+initializeStateDirectory(true, true);
+
+directory.getOrCreateDirectoryForTask(new TaskId(0, 0, "topology1"));
+directory.getOrCreateDirectoryForTask(new TaskId(0, 1, "topology1"));
+directory.getOrCreateDirectoryForTask(new TaskId(0, 0, "topology2"));
+
+assertThat(new File(appDir, "__topology1__").exists(), is(true));
+assertThat(new File(appDir, "__topology1__").isDirectory(), is(true));
+assertThat(new File(appDir, "__topology2__").exists(), is(true));
+assertThat(new File(appDir, "__topology2__").isDirectory(), is(true));
+
+assertThat(new File(new File(appDir, "__topology1__"),
"0_0").exists(), is(true));
+assertThat(new File(new File(appDir, "__topology1__"),
"0_0").isDirectory(), is(true));
+assertThat(new File(new File(appDir, "__topology1__"),
"0_1").exists(), is(true));
+assertThat(new File(new File(appDir, "__topology1__"),
"0_1").isDirectory(), is(true));
+assertThat(new File(new File(appDir, "__topology2__"),
"0_0").exists(), is(true));
+assertThat(new File(new File(appDir, "__topology2__"),
"0_0").isDirectory(), is(true));
+}
+
+@Test
+public void shouldOnlyListNonEmptyTaskDirectoriesInNamedTopologies()
throws IOException {
+initializeStateDirectory(true, true);
+
+TestUtils.tempDirectory(appDir.toPath(), "foo");
+final TaskDirectory taskDir1 = new
TaskDirectory(directory.getOrCreateDirectoryForTask(new TaskId(0, 0,
"topology1")), "topology1");
+final TaskDirectory taskDir2 = new
TaskDirectory(directory.getOrCreateDirectoryForTask(new TaskId(0, 1,
"topology1")), "topology1");
+final TaskDirectory taskDir3 = new
TaskDirectory(directory.getOrCreateDirectoryForTask(new TaskId(0, 0,
"topology2")), "topology2");
+
+final File storeDir = new File(taskDir1.file(), "store");
+assertTrue(storeDir.mkdir());
+
+assertThat(new HashSet<>(directory.listAllTaskDirectories()),
equalTo(mkSet(taskDir1, taskDir2, taskDir3)));
+assertThat(directory.listNonEmptyTaskDirectories(),
equalTo(singletonList(taskDir1)));
+
+Utils.delete(taskDir1.file());
+
+assertThat(new HashSet<>(directory.listAllTaskDirectories()),
equalTo(mkSet(taskDir2, taskDir3)));
+assertThat(directory.listNonEmptyTaskDirectories(),
equalTo(emptyList()));
+}
+
+@Test
+public void shouldRemoveNonEmptyNamedTopologyDirsWhenCallingClean() throws
Exception {
+initializeStateDirectory(true, true);
+final File taskDir = directory.getOrCreateDirectoryForTask(new
TaskId(2, 0, "topology1"));
+final File namedTopologyDir = new File(appDir, "__topology1__");
+
+assertThat(taskDir.exists(), is(true));
+assertThat(namedTopologyDir.exists(), is(true));
+directory.clean();
+assertThat(taskDir.exists(), is(false));
+assertThat(namedTopologyDir.exists(), is(false));
+}
+
+@Test
+public void shouldRemoveEmptyNamedTopologyDirsWhenCallingClean() throws
IOException {
+initializeStateDirectory(true, true);
+final File namedTopologyDir = new File(appDir, "__topology1__");
+assertThat(namedTopologyDir.mkdir(), is(true));
+assertThat(namedTopologyDir.exists(), is(true));
+directory.clean();
+assertThat(namedTopologyDir.exists(), is(false));
+}
+
+@Test
+public void
shouldNotRemoveDirsThatDoNotMatchNamedTopologyDirsWhenCallingClean() throws
IOException {
+initializeStateDirectory(true, true);
+final File someDir = new File(appDir,
"_not-a-valid-named-topology_dir_name_");
+assertThat(someDir.mkdir(), is(true));
+assertThat(someDir.exists(), is(true));
+directory.clean();
+assertThat(someDir.exists(), is(true));
+}
+
+@Test
+public void
shouldCleanupObsoleteTaskDirectoriesInNamedTopologiesAndDeleteTheParentDirectories()
throws IOException {
Review comment:
Could we add a test case to verify that in case both named topology dir
and non-named topology dir co-exist, we would at certain step check against and
throw?
--
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] dejan2609 commented on pull request #10698: KAFKA-12770: introduce `checkstyleVersion` build option (for overriding CheckStyle project-defined dependency version)
dejan2609 commented on pull request #10698: URL: https://github.com/apache/kafka/pull/10698#issuecomment-853408053 Just tagging @ijuma here again (to come back up to the surface). -- 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 commented on a change in pull request #10609: KAFKA-12648: Pt. 1 - Add NamedTopology to protocol and state directory structure
guozhangwang commented on a change in pull request #10609:
URL: https://github.com/apache/kafka/pull/10609#discussion_r644344196
##
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/StateDirectory.java
##
@@ -462,39 +512,49 @@ private void cleanRemovedTasksCalledByUser() throws
Exception {
* List all of the task directories that are non-empty
* @return The list of all the non-empty local directories for stream tasks
*/
-File[] listNonEmptyTaskDirectories() {
-final File[] taskDirectories;
-if (!hasPersistentStores || !stateDir.exists()) {
-taskDirectories = new File[0];
-} else {
-taskDirectories =
-stateDir.listFiles(pathname -> {
-if (!pathname.isDirectory() ||
!TASK_DIR_PATH_NAME.matcher(pathname.getName()).matches()) {
-return false;
-} else {
-return !taskDirIsEmpty(pathname);
-}
-});
-}
-
-return taskDirectories == null ? new File[0] : taskDirectories;
+List listNonEmptyTaskDirectories() {
+return listTaskDirectories(pathname -> {
+if (!pathname.isDirectory() ||
!TASK_DIR_PATH_NAME.matcher(pathname.getName()).matches()) {
+return false;
+} else {
+return !taskDirIsEmpty(pathname);
+}
+});
}
/**
- * List all of the task directories
+ * List all of the task directories along with their parent directory if
they belong to a named topology
* @return The list of all the existing local directories for stream tasks
*/
-File[] listAllTaskDirectories() {
-final File[] taskDirectories;
-if (!hasPersistentStores || !stateDir.exists()) {
-taskDirectories = new File[0];
-} else {
-taskDirectories =
-stateDir.listFiles(pathname -> pathname.isDirectory()
- &&
TASK_DIR_PATH_NAME.matcher(pathname.getName()).matches());
+List listAllTaskDirectories() {
+return listTaskDirectories(pathname -> pathname.isDirectory() &&
TASK_DIR_PATH_NAME.matcher(pathname.getName()).matches());
+}
+
+private List listTaskDirectories(final FileFilter filter) {
+final List taskDirectories = new ArrayList<>();
+if (hasPersistentStores && stateDir.exists()) {
+if (hasNamedTopologies) {
Review comment:
I was asking more for a semantic one -- as long as this is not expected
then I'm happy for this piece as is :)
--
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 commented on a change in pull request #10609: KAFKA-12648: Pt. 1 - Add NamedTopology to protocol and state directory structure
guozhangwang commented on a change in pull request #10609:
URL: https://github.com/apache/kafka/pull/10609#discussion_r644343550
##
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/SubscriptionInfo.java
##
@@ -125,6 +130,29 @@ public int errorCode() {
return data.errorCode();
}
+// For version > MIN_NAMED_TOPOLOGY_VERSION
+private void
setTaskOffsetSumDataWithNamedTopologiesFromTaskOffsetSumMap(final Map taskOffsetSums) {
+final Map>
topicGroupIdToPartitionOffsetSum = new HashMap<>();
+for (final Map.Entry taskEntry :
taskOffsetSums.entrySet()) {
+final TaskId task = taskEntry.getKey();
+
topicGroupIdToPartitionOffsetSum.computeIfAbsent(task.topicGroupId, t -> new
ArrayList<>()).add(
+new SubscriptionInfoData.PartitionToOffsetSum()
+.setPartition(task.partition)
+.setOffsetSum(taskEntry.getValue()));
+}
+
+data.setTaskOffsetSums(taskOffsetSums.entrySet().stream().map(t -> {
+final SubscriptionInfoData.TaskOffsetSum taskOffsetSum = new
SubscriptionInfoData.TaskOffsetSum();
+final TaskId task = t.getKey();
+taskOffsetSum.setTopicGroupId(task.topicGroupId);
+taskOffsetSum.setPartition(task.partition);
Review comment:
Thanks for the explanation!
--
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-4793) Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed tasks
[ https://issues.apache.org/jira/browse/KAFKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355976#comment-17355976 ] Kalpesh Patel commented on KAFKA-4793: -- (y) > Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed > tasks > - > > Key: KAFKA-4793 > URL: https://issues.apache.org/jira/browse/KAFKA-4793 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Gwen Shapira >Assignee: Kalpesh Patel >Priority: Major > Labels: needs-kip > > Sometimes tasks stop due to repeated failures. Users will want to restart the > connector and have it retry after fixing an issue. > We expected "POST /connectors/(string: name)/restart" to cause retry of > failed tasks, but this doesn't appear to be the case. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (KAFKA-4793) Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed tasks
[ https://issues.apache.org/jira/browse/KAFKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kalpesh Patel reassigned KAFKA-4793: Assignee: Kalpesh Patel (was: Randall Hauch) > Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed > tasks > - > > Key: KAFKA-4793 > URL: https://issues.apache.org/jira/browse/KAFKA-4793 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Gwen Shapira >Assignee: Kalpesh Patel >Priority: Major > Labels: needs-kip > > Sometimes tasks stop due to repeated failures. Users will want to restart the > connector and have it retry after fixing an issue. > We expected "POST /connectors/(string: name)/restart" to cause retry of > failed tasks, but this doesn't appear to be the case. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (KAFKA-7815) SourceTask should expose ACK'd offsets, metadata
[ https://issues.apache.org/jira/browse/KAFKA-7815?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ryanne Dolan resolved KAFKA-7815. - Fix Version/s: 2.4.0 Resolution: Fixed Equivalent functionality was included as part of KIP-382. > SourceTask should expose ACK'd offsets, metadata > > > Key: KAFKA-7815 > URL: https://issues.apache.org/jira/browse/KAFKA-7815 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Ryanne Dolan >Assignee: Ryanne Dolan >Priority: Minor > Labels: needs-kip > Fix For: 2.4.0 > > Original Estimate: 24h > Remaining Estimate: 24h > > Add a new callback method, recordLogged(), to notify SourceTasks when a > record is ACK'd by the downstream broker. Include offsets and metadata of > ACK'd record. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-4793) Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed tasks
[ https://issues.apache.org/jira/browse/KAFKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355973#comment-17355973 ] Randall Hauch commented on KAFKA-4793: -- [~kpatelatwork], thanks for offering. Yes, your help would be very much welcomed. I can work with you offline about the work I've already started but haven't yet completed. Feel free to assign this issue to yourself. > Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed > tasks > - > > Key: KAFKA-4793 > URL: https://issues.apache.org/jira/browse/KAFKA-4793 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Gwen Shapira >Assignee: Randall Hauch >Priority: Major > Labels: needs-kip > > Sometimes tasks stop due to repeated failures. Users will want to restart the > connector and have it retry after fixing an issue. > We expected "POST /connectors/(string: name)/restart" to cause retry of > failed tasks, but this doesn't appear to be the case. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-4793) Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed tasks
[ https://issues.apache.org/jira/browse/KAFKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355967#comment-17355967 ] Kalpesh Patel commented on KAFKA-4793: -- [~rhauch] this is a great proposal, would it be possible for me to pick up the implementation of this KIP? > Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed > tasks > - > > Key: KAFKA-4793 > URL: https://issues.apache.org/jira/browse/KAFKA-4793 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Gwen Shapira >Assignee: Randall Hauch >Priority: Major > Labels: needs-kip > > Sometimes tasks stop due to repeated failures. Users will want to restart the > connector and have it retry after fixing an issue. > We expected "POST /connectors/(string: name)/restart" to cause retry of > failed tasks, but this doesn't appear to be the case. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (KAFKA-12881) Consider Un-Deprecation of Consumer#committed methods
Josep Prat created KAFKA-12881: -- Summary: Consider Un-Deprecation of Consumer#committed methods Key: KAFKA-12881 URL: https://issues.apache.org/jira/browse/KAFKA-12881 Project: Kafka Issue Type: Task Components: clients Reporter: Josep Prat During KAFKA-8880, following [https://cwiki.apache.org/confluence/display/KAFKA/KIP-520%3A+Add+overloaded+Consumer%23committed+for+batching+partitions,] methods _org.apache.kafka.clients.consumer.Consumer#committed(org.apache.kafka.common.TopicPartition)_ and _org.apache.kafka.clients.consumer.Consumer#committed(org.apache.kafka.common.TopicPartition, java.time.Duration)_ were deprecated. As both methods are still widely used, it might be worth to either remove the deprecation for mentioned methods, or provide a deeper reasoning on why they should stay deprecated and eventually removed. If the later is decided, then the original KIP should be updated to include said reasoning. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] cmccabe commented on pull request #10787: KAFKA-12864: Move KafkaEventQueue into server-common
cmccabe commented on pull request #10787: URL: https://github.com/apache/kafka/pull/10787#issuecomment-853338511 OK. Thanks for the reviews. I will commit after Jenkins runs if there are no objections (current PR just moves KafkaEventQueue) -- 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] [Updated] (KAFKA-12864) Move KafkaEventQueue into server-common
[ https://issues.apache.org/jira/browse/KAFKA-12864?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Colin McCabe updated KAFKA-12864: - Summary: Move KafkaEventQueue into server-common (was: Move KafkaEventQueue and timeline data structures into server-common) > Move KafkaEventQueue into server-common > --- > > Key: KAFKA-12864 > URL: https://issues.apache.org/jira/browse/KAFKA-12864 > Project: Kafka > Issue Type: Improvement >Reporter: Colin McCabe >Assignee: Colin McCabe >Priority: Minor > > Move KafkaEventQueue and timeline data structures into server-common -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12864) Move KafkaEventQueue into server-common
[ https://issues.apache.org/jira/browse/KAFKA-12864?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Colin McCabe updated KAFKA-12864: - Description: Move KafkaEventQueue into server-common (was: Move KafkaEventQueue and timeline data structures into server-common) > Move KafkaEventQueue into server-common > --- > > Key: KAFKA-12864 > URL: https://issues.apache.org/jira/browse/KAFKA-12864 > Project: Kafka > Issue Type: Improvement >Reporter: Colin McCabe >Assignee: Colin McCabe >Priority: Minor > > Move KafkaEventQueue into server-common -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] ijuma commented on pull request #10787: KAFKA-12864: Move KafkaEventQueue into server-common
ijuma commented on pull request #10787: URL: https://github.com/apache/kafka/pull/10787#issuecomment-853338002 Makes sense. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #10787: KAFKA-12864: Move queue and timeline into server-common
cmccabe commented on pull request #10787: URL: https://github.com/apache/kafka/pull/10787#issuecomment-853335116 > I would personally not move these classes unless we believe we will use them from other modules. They're a bit specialized, especially the timeline ones. `KafkaEventQueue` is already used in `core`. It's just an event queue; there is nothing metadata-specific about it, so I think it belongs in `server-common`. The timeline data structures are not used anywhere else, however. I will revise the PR to move just the queue and not the timeline data structures. -- 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 pull request #10787: KAFKA-12864: Move queue and timeline into server-common
ijuma commented on pull request #10787: URL: https://github.com/apache/kafka/pull/10787#issuecomment-853323788 I would personally not move these classes unless we believe we will use them from other modules. They're a bit specialized, especially the timeline ones. -- 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 pull request #10787: KAFKA-12864: Move queue and timeline into server-common
ijuma commented on pull request #10787: URL: https://github.com/apache/kafka/pull/10787#issuecomment-853316612 Are we using these classes from any module besides metadata atm? -- 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-12867) Trogdor ConsumeBenchWorker quits prematurely with maxMessages config
[ https://issues.apache.org/jira/browse/KAFKA-12867?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Rajini Sivaram resolved KAFKA-12867. Fix Version/s: 3.0.0 Reviewer: Rajini Sivaram Resolution: Fixed > Trogdor ConsumeBenchWorker quits prematurely with maxMessages config > > > Key: KAFKA-12867 > URL: https://issues.apache.org/jira/browse/KAFKA-12867 > Project: Kafka > Issue Type: Bug >Reporter: Kowshik Prakasam >Assignee: Kowshik Prakasam >Priority: Major > Fix For: 3.0.0 > > > The trogdor > [ConsumeBenchWorker|https://github.com/apache/kafka/commits/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java] > has a bug. If one of the consumption tasks completes executing successfully > due to [maxMessages being > consumed|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L245], > then, the consumption task [notifies the > doneFuture|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L285] > causing the ConsumeBenchWorker to halt. This becomes a problem when more > than 1 consumption task is running in parallel, because the successful > completion of 1 of the tasks shuts down the entire worker while the other > tasks are still running. When the worker is shut down, it > [kills|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L482] > all the active consumption tasks, which is not the desired behavior. > The fix is to not notify the doneFuture when 1 of the consumption tasks > complete without error. Instead, we should defer the notification to the > [CloseStatusUpdater|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L299] > thread. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Assigned] (KAFKA-12867) Trogdor ConsumeBenchWorker quits prematurely with maxMessages config
[ https://issues.apache.org/jira/browse/KAFKA-12867?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Rajini Sivaram reassigned KAFKA-12867: -- Assignee: Kowshik Prakasam > Trogdor ConsumeBenchWorker quits prematurely with maxMessages config > > > Key: KAFKA-12867 > URL: https://issues.apache.org/jira/browse/KAFKA-12867 > Project: Kafka > Issue Type: Bug >Reporter: Kowshik Prakasam >Assignee: Kowshik Prakasam >Priority: Major > > The trogdor > [ConsumeBenchWorker|https://github.com/apache/kafka/commits/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java] > has a bug. If one of the consumption tasks completes executing successfully > due to [maxMessages being > consumed|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L245], > then, the consumption task [notifies the > doneFuture|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L285] > causing the ConsumeBenchWorker to halt. This becomes a problem when more > than 1 consumption task is running in parallel, because the successful > completion of 1 of the tasks shuts down the entire worker while the other > tasks are still running. When the worker is shut down, it > [kills|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L482] > all the active consumption tasks, which is not the desired behavior. > The fix is to not notify the doneFuture when 1 of the consumption tasks > complete without error. Instead, we should defer the notification to the > [CloseStatusUpdater|https://github.com/apache/kafka/blob/fc405d792de12a50956195827eaf57bbf6c9/trogdor/src/main/java/org/apache/kafka/trogdor/workload/ConsumeBenchWorker.java#L299] > thread. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] ijuma merged pull request #10808: KAFKA-12880: Remove deprecated `Count` and `SampledTotal` in 3.0
ijuma merged pull request #10808: URL: https://github.com/apache/kafka/pull/10808 -- 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] rajinisivaram merged pull request #10797: KAFKA-12867: Fix ConsumeBenchWorker exit behavior for maxMessages config
rajinisivaram merged pull request #10797: URL: https://github.com/apache/kafka/pull/10797 -- 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 pull request #10808: KAFKA-12880: Remove deprecated `Count` and `SampledTotal` in 3.0
ijuma commented on pull request #10808: URL: https://github.com/apache/kafka/pull/10808#issuecomment-853284235 One unrelated failure: > Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.integration.ConnectorClientPolicyIntegrationTest.testCreateWithNotAllowedOverridesForPrincipalPolicy -- 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] vvcephei commented on a change in pull request #10731: KAFKA-12815: Update JavaDocs of ValueTransformerWithKey
vvcephei commented on a change in pull request #10731:
URL: https://github.com/apache/kafka/pull/10731#discussion_r644196594
##
File path:
streams/src/main/java/org/apache/kafka/streams/kstream/ValueTransformerWithKey.java
##
@@ -75,14 +75,20 @@
void init(final ProcessorContext context);
/**
- * Transform the given [key and ]value to a new value.
+ * Transform the given [key and] value to a new value.
* Additionally, any {@link StateStore} that is {@link
KStream#transformValues(ValueTransformerWithKeySupplier, String...)
* attached} to this operator can be accessed and modified arbitrarily (cf.
* {@link ProcessorContext#getStateStore(String)}).
*
- * Note, that using {@link ProcessorContext#forward(Object, Object)} or
+ * Note that using {@link ProcessorContext#forward(Object, Object)} or
* {@link ProcessorContext#forward(Object, Object, To)} is not allowed
within {@code transform} and
* will result in an {@link StreamsException exception}.
+ *
+ * Note that if a {@code ValueTransformerWithKey} is used in a {@link
KTable#transformValues(ValueTransformerWithKeySupplier, String...)}
+ * (or any other overload of {@code KTable#transformValues(...)})
operation,
+ * then the provided {@link ProcessorContext} from {@link
#init(ProcessorContext)}
+ * does not guarantee that all context information will be available when
{@code transform()}
+ * is executed.
Review comment:
All I was trying to do was characterize why the context information
might be missing. If there are _no_ out-of-band operations, then there should
never be missing context.
--
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] tombentley commented on pull request #10807: KAFKA-12797: Log the evictor of fetch sessions
tombentley commented on pull request #10807: URL: https://github.com/apache/kafka/pull/10807#issuecomment-853251332 @weeco you're right it's not super helpful, because you would have to post-process the log to figure out who was causing the problem (i.e. observe that the same principal and client id were the evictor for many successive evictions). However, this patch adds no new logging, so it doesn't really make 2 much worse than it already may be, while at least allowing the problem user to _be_ identified. So I think it's a simple improvement over the current situation, where it's basically impossible to identify the culprit. This PR doesn't preclude finding a better solution later on. The issue with metrics like `IncrementalFetchSessionsCreatedPerSec` is that that could be a lot of metrics, which most of the time are useless. I don't feel that adding and maintaining that many metrics can really be justified for an issue which is very rarely seen in practice. -- 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 #10668: MINOR: Apply try-with-resource to KafkaStreamsTest
mjsax commented on pull request #10668: URL: https://github.com/apache/kafka/pull/10668#issuecomment-853249482 @vitojeng -- Seems there is some conflicts. Can you rebase your PR so we can merge it? -- 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 a change in pull request #10668: MINOR: Apply try-with-resource to KafkaStreamsTest
mjsax commented on a change in pull request #10668:
URL: https://github.com/apache/kafka/pull/10668#discussion_r644184061
##
File path: streams/src/test/java/org/apache/kafka/streams/KafkaStreamsTest.java
##
@@ -491,25 +493,23 @@ public void testStateThreadClose() throws Exception {
() -> streams.localThreadsMetadata().stream().allMatch(t ->
t.threadState().equals("DEAD")),
"Streams never stopped"
);
-} finally {
streams.close();
Review comment:
I missed the fact that we moved the `waitForCondition` check _inside_ of
the try-catch block... For this case, we need to call `close` explicitly of
course, as we are still in the block and `close()` is not auto-called yet...
Sorry for the confusion.
--
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] weeco commented on pull request #10807: KAFKA-12797: Log the evictor of fetch sessions
weeco commented on pull request #10807: URL: https://github.com/apache/kafka/pull/10807#issuecomment-853223533 Hey @tombentley , thanks for tackling this! I understand your performance concerns in regards to the quota implementation; I can't judge if a quota for that is actually too expensive. However here are my thoughts regarding your PR: Unless I missunderstand your proposed changes here, I believe that logging the fetch session slot evictor is currently not super helpful because: 1. I believe fetch sessions are expected to be evicted. For me it seems like fetch sessions are not proactively relived from the session slots cache. In my clusters I can see the fetch session slots slowly growing over time (hours until all slots are taken) and then fetch sessions are regularly evicted at a rate of 1-3 evictions / second. I noticed this by tracking this via the JMX metrics `IncrementalFetchSessionEvictionsPerSec` and `NumIncrementalFetchSessions` (introduced in KIP-227) 2. A potential attacker (or misbehaving client such as sarama v1.26.0) can quickly create new fetch sessions (thousands of fetch sessions per seconds) which would then cause one log line for each eviction. This again can cause high CPU usage (for printing the log line) or at least cause problems for many logging pipelines. **I could envision two other solutions to mitigate the issue around leaking fetch sessions:** 1. Create one fetch session slot cache per client with a fixed, relatively low number of available fetch sessions. This way a single client can not negatively impact other clients. Possibly this is even a performance improvement because the lock contention as required in one large cache that is concurrently accessed from many fetch different fetch requests may be reduced? 2. Introduce a new counter metric that logs the number of fetch sessions created per client id. Something like `IncrementalFetchSessionsCreatedPerSec` -- 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] jsancio commented on a change in pull request #10749: KAFKA-12773: Use UncheckedIOException when wrapping IOException
jsancio commented on a change in pull request #10749:
URL: https://github.com/apache/kafka/pull/10749#discussion_r644156023
##
File path: raft/src/main/java/org/apache/kafka/snapshot/Snapshots.java
##
@@ -68,15 +68,27 @@ public static Path snapshotPath(Path logDir, OffsetAndEpoch
snapshotId) {
return snapshotDir(logDir).resolve(filenameFromSnapshotId(snapshotId)
+ SUFFIX);
}
-public static Path createTempFile(Path logDir, OffsetAndEpoch snapshotId)
throws IOException {
+public static Path createTempFile(Path logDir, OffsetAndEpoch snapshotId) {
Path dir = snapshotDir(logDir);
+Path tempFile;
-// Create the snapshot directory if it doesn't exists
-Files.createDirectories(dir);
-
-String prefix = String.format("%s-",
filenameFromSnapshotId(snapshotId));
+try {
+// Create the snapshot directory if it doesn't exists
+Files.createDirectories(dir);
-return Files.createTempFile(dir, prefix, PARTIAL_SUFFIX);
+String prefix = String.format("%s-",
filenameFromSnapshotId(snapshotId));
+tempFile = Files.createTempFile(dir, prefix, PARTIAL_SUFFIX);
+} catch (IOException e) {
+throw new UncheckedIOException(
+String.format(
+"Error creating temporary file. logDir = %s,
snapshotId = %s",
+dir.toAbsolutePath(),
+snapshotId
+),
+e
Review comment:
Some of these lines have extra 4 space. Other lines have extra 8 spaces.
##
File path:
raft/src/main/java/org/apache/kafka/snapshot/FileRawSnapshotReader.java
##
@@ -75,17 +75,27 @@ public void close() {
*
* @param logDir the directory for the topic partition
* @param snapshotId the end offset and epoch for the snapshotId
- * @throws java.nio.file.NoSuchFileException if the snapshot doesn't exist
- * @throws IOException for any IO error while opening the snapshot
*/
-public static FileRawSnapshotReader open(Path logDir, OffsetAndEpoch
snapshotId) throws IOException {
-FileRecords fileRecords = FileRecords.open(
-Snapshots.snapshotPath(logDir, snapshotId).toFile(),
-false, // mutable
-true, // fileAlreadyExists
-0, // initFileSize
-false // preallocate
-);
+public static FileRawSnapshotReader open(Path logDir, OffsetAndEpoch
snapshotId) {
+FileRecords fileRecords;
+Path filePath = Snapshots.snapshotPath(logDir, snapshotId);
+try {
+fileRecords = FileRecords.open(
+filePath.toFile(),
+false, // mutable
+true, // fileAlreadyExists
+0, // initFileSize
+false // preallocate
Review comment:
These 5 lines have extra 4 spaces of indentation.
##
File path:
raft/src/main/java/org/apache/kafka/snapshot/FileRawSnapshotReader.java
##
@@ -75,17 +75,27 @@ public void close() {
*
* @param logDir the directory for the topic partition
* @param snapshotId the end offset and epoch for the snapshotId
- * @throws java.nio.file.NoSuchFileException if the snapshot doesn't exist
- * @throws IOException for any IO error while opening the snapshot
*/
-public static FileRawSnapshotReader open(Path logDir, OffsetAndEpoch
snapshotId) throws IOException {
-FileRecords fileRecords = FileRecords.open(
-Snapshots.snapshotPath(logDir, snapshotId).toFile(),
-false, // mutable
-true, // fileAlreadyExists
-0, // initFileSize
-false // preallocate
-);
+public static FileRawSnapshotReader open(Path logDir, OffsetAndEpoch
snapshotId) {
+FileRecords fileRecords;
+Path filePath = Snapshots.snapshotPath(logDir, snapshotId);
+try {
+fileRecords = FileRecords.open(
+filePath.toFile(),
+false, // mutable
+true, // fileAlreadyExists
+0, // initFileSize
+false // preallocate
+);
+} catch (IOException e) {
+throw new UncheckedIOException(
+String.format(
+"Unable to Opens a snapshot file %s",
+filePath.toAbsolutePath()
Review comment:
These 2 lines have extra 4 spaces of indentation.
##
File path:
raft/src/main/java/org/apache/kafka/snapshot/FileRawSnapshotWriter.java
##
@@ -58,7 +59,11 @@ public long sizeInBytes() {
try {
return channel.size();
} catch (IOException e) {
-throw new RuntimeException(e);
+throw new UncheckedIOException(
+String.format("Error
[jira] [Commented] (KAFKA-4793) Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed tasks
[ https://issues.apache.org/jira/browse/KAFKA-4793?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355860#comment-17355860 ] Randall Hauch commented on KAFKA-4793: -- I've created a KIP that expands the existing REST API method to restart a connector (the `Connector` instance, not the "named connector") with two new optional query parameters. Users can make one call using this method (with the query parameters) to request a combination of failed/all `Connector` and/or `Task` instances. https://cwiki.apache.org/confluence/display/KAFKA/KIP-745%3A+Connect+API+to+restart+connector+and+tasks > Kafka Connect: POST /connectors/(string: name)/restart doesn't start failed > tasks > - > > Key: KAFKA-4793 > URL: https://issues.apache.org/jira/browse/KAFKA-4793 > Project: Kafka > Issue Type: Improvement > Components: KafkaConnect >Reporter: Gwen Shapira >Assignee: Randall Hauch >Priority: Major > Labels: needs-kip > > Sometimes tasks stop due to repeated failures. Users will want to restart the > connector and have it retry after fixing an issue. > We expected "POST /connectors/(string: name)/restart" to cause retry of > failed tasks, but this doesn't appear to be the case. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] jsancio commented on a change in pull request #10749: KAFKA-12773: Use UncheckedIOException when wrapping IOException
jsancio commented on a change in pull request #10749:
URL: https://github.com/apache/kafka/pull/10749#discussion_r644148707
##
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##
@@ -372,27 +371,23 @@ private void maybeFireLeaderChange() {
@Override
public void initialize() {
-try {
-quorum.initialize(new OffsetAndEpoch(log.endOffset().offset,
log.lastFetchedEpoch()));
+quorum.initialize(new OffsetAndEpoch(log.endOffset().offset,
log.lastFetchedEpoch()));
-long currentTimeMs = time.milliseconds();
-if (quorum.isLeader()) {
-throw new IllegalStateException("Voter cannot initialize as a
Leader");
-} else if (quorum.isCandidate()) {
-onBecomeCandidate(currentTimeMs);
-} else if (quorum.isFollower()) {
-onBecomeFollower(currentTimeMs);
-}
+long currentTimeMs = time.milliseconds();
+if (quorum.isLeader()) {
+throw new IllegalStateException("Voter cannot initialize as a
Leader");
+} else if (quorum.isCandidate()) {
+onBecomeCandidate(currentTimeMs);
+} else if (quorum.isFollower()) {
+onBecomeFollower(currentTimeMs);
+}
-// When there is only a single voter, become candidate immediately
-if (quorum.isVoter()
-&& quorum.remoteVoters().isEmpty()
-&& !quorum.isCandidate()) {
+// When there is only a single voter, become candidate immediately
+if (quorum.isVoter()
+&& quorum.remoteVoters().isEmpty()
+&& !quorum.isCandidate()) {
-transitionToCandidate(currentTimeMs);
-}
-} catch (IOException e) {
-throw new RuntimeException(e);
+transitionToCandidate(currentTimeMs);
Review comment:
Yeah, I think it is fine as is. I was just pointing out that it is
technically different from the original code.
--
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] ryannedolan commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
ryannedolan commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r644125908
##
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##
@@ -58,7 +58,11 @@ import scala.util.{Failure, Success, Try}
*enable.auto.commit=false
* 3. Mirror Maker Setting:
*abort.on.send.failure=true
+ *
+ * @deprecated The original Mirror Maker is deprecated since release 3.0.
Similar functionality can be
+ *found in the Connect-based re-implementation by the same name (aka MM2).
Review comment:
Looking at other uses of the @deprecated javadoc tags, I see comments
like "Since 2.4, use the PartitionReassignment Kafka API instead", which isn't
far off from your suggestion. I'll reword, 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[jira] [Commented] (KAFKA-12436) deprecate MirrorMaker v1
[ https://issues.apache.org/jira/browse/KAFKA-12436?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355841#comment-17355841 ] Ryanne Dolan commented on KAFKA-12436: -- [~ijuma] can I get a review please? > deprecate MirrorMaker v1 > > > Key: KAFKA-12436 > URL: https://issues.apache.org/jira/browse/KAFKA-12436 > Project: Kafka > Issue Type: Improvement > Components: mirrormaker >Reporter: Ryanne Dolan >Assignee: Ryanne Dolan >Priority: Minor > Fix For: 3.0.0 > > > Per KIP-382, the old MirrorMaker code (MM1) should be deprecated and > subsequently removed. Targeting upcoming release 3.0.0, we should mark > mirror-maker as deprecated, but leave code in place until subsequent major > release (4.0.0). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Updated] (KAFKA-12436) deprecate MirrorMaker v1
[ https://issues.apache.org/jira/browse/KAFKA-12436?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ryanne Dolan updated KAFKA-12436: - Reviewer: Ismael Juma (was: Konstantine Karantasis) > deprecate MirrorMaker v1 > > > Key: KAFKA-12436 > URL: https://issues.apache.org/jira/browse/KAFKA-12436 > Project: Kafka > Issue Type: Improvement > Components: mirrormaker >Reporter: Ryanne Dolan >Assignee: Ryanne Dolan >Priority: Minor > Fix For: 3.0.0 > > > Per KIP-382, the old MirrorMaker code (MM1) should be deprecated and > subsequently removed. Targeting upcoming release 3.0.0, we should mark > mirror-maker as deprecated, but leave code in place until subsequent major > release (4.0.0). -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-10900) Add metrics enumerated in KIP-630
[ https://issues.apache.org/jira/browse/KAFKA-10900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355728#comment-17355728 ] loboxu edited comment on KAFKA-10900 at 6/2/21, 3:52 PM: - [~jagsancio] Can you give me some guidance? Where the metrics are added in the code. I have looked at the code and the logic is not very clear. Here is my understanding of the position: * GenSnapshotLatencyMs：ReplicatedCounter.handleCommit() - LoadSnapshotLatencyMs：ReplicatedCounter.handleSnapshot(() - SnapshotSizeBytes: ReplicatedCounter.handleCommit() - SnapshotLag: ReplicatedCounter.handleCommit() was (Author: loboxu): [~jagsancio] Can you give me some guidance? Where the metrics are added in the code. I have looked at the code and the logic is not very clear. Here is my understanding of the position: * GenSnapshotLatencyMs：KafkaMetadataLog.createSnapshot() - LoadSnapshotLatencyMs：KafkaMetadataLog.readSnapshot() - SnapshotSizeBytes: KafkaMetadataLog.createSnapshot() - SnapshotLag: KafkaMetadataLog.appendAsLeader() > Add metrics enumerated in KIP-630 > - > > Key: KAFKA-10900 > URL: https://issues.apache.org/jira/browse/KAFKA-10900 > Project: Kafka > Issue Type: Sub-task > Components: replication >Reporter: Jose Armando Garcia Sancio >Assignee: loboxu >Priority: Major > > KIP-630 enumerates a few metrics. Makes sure that those metrics are > implemented. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] satishd commented on a change in pull request #10733: KAFKA-12816 Added tiered storage related configs including remote log manager configs.
satishd commented on a change in pull request #10733:
URL: https://github.com/apache/kafka/pull/10733#discussion_r636298299
##
File path:
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java
##
@@ -0,0 +1,323 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.server.log.remote.storage;
+
+import org.apache.kafka.common.config.AbstractConfig;
+import org.apache.kafka.common.config.ConfigDef;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+import static org.apache.kafka.common.config.ConfigDef.Importance.LOW;
+import static org.apache.kafka.common.config.ConfigDef.Importance.MEDIUM;
+import static org.apache.kafka.common.config.ConfigDef.Range.atLeast;
+import static org.apache.kafka.common.config.ConfigDef.Type.BOOLEAN;
+import static org.apache.kafka.common.config.ConfigDef.Type.INT;
+import static org.apache.kafka.common.config.ConfigDef.Type.LONG;
+import static org.apache.kafka.common.config.ConfigDef.Type.STRING;
+
+public final class RemoteLogManagerConfig {
+
+/**
+ * Prefix used for properties to be passed to {@link RemoteStorageManager}
implementation. Remote log subsystem collects all the properties having
+ * this prefix and passed to {@code RemoteStorageManager} using {@link
RemoteStorageManager#configure(Map)}.
+ */
+public static final String REMOTE_STORAGE_MANAGER_CONFIG_PREFIX =
"remote.log.storage.manager.impl.";
Review comment:
I will update KIP about the prefixes for both `RemoteStorageManager` and
`RemoteLogMetadataManager` properties. These values are intended to be the
config property, I will update the PR addressing this behacior.
--
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-12847) Dockerfile needed for kafka system tests needs changes
[
https://issues.apache.org/jira/browse/KAFKA-12847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355801#comment-17355801
]
Chia-Ping Tsai commented on KAFKA-12847:
{quote}
ARG UID="1000" ==> Has no effect as UID value is read-only in bash
{quote}
The UID can be override by
https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak#L231
{quote}
// Error during docker build => useradd: UID 0 is not unique, root user id is 0
{quote}
Could you check the UID in your machine? For example: `echo $UID`. It seems
your local machine does not have variable UID.
> Dockerfile needed for kafka system tests needs changes
> --
>
> Key: KAFKA-12847
> URL: https://issues.apache.org/jira/browse/KAFKA-12847
> Project: Kafka
> Issue Type: Bug
> Components: system tests
>Affects Versions: 2.8.0, 2.7.1
> Environment: Issue tested in environments below but is independent of
> h/w arch. or Linux flavor: -
> 1.) RHEL-8.3 on x86_64
> 2.) RHEL-8.3 on IBM Power (ppc64le)
> 3.) apache/kafka branch tested: trunk (master)
>Reporter: Abhijit Mane
>Assignee: Abhijit Mane
>Priority: Major
> Labels: easyfix
> Attachments: Dockerfile.upstream
>
>
> Hello,
> I tried apache/kafka system tests as per documentation: -
> ([https://github.com/apache/kafka/tree/trunk/tests#readme|https://github.com/apache/kafka/tree/trunk/tests#readme_])
> =
> PROBLEM
> ~~
> 1.) As root user, clone kafka github repo and start "kafka system tests"
> # git clone [https://github.com/apache/kafka.git]
> # cd kafka
> # ./gradlew clean systemTestLibs
> # bash tests/docker/run_tests.sh
> 2.) Dockerfile issue -
> [https://github.com/apache/kafka/blob/trunk/tests/docker/Dockerfile]
> This file has an *UID* entry as shown below: -
> ---
> ARG *UID*="1000"
> RUN useradd -u $*UID* ducker
> // {color:#de350b}*Error during docker build*{color} => useradd: UID 0 is not
> unique, root user id is 0
> ---
> I ran everything as root which means the built-in bash environment variable
> 'UID' always
> resolves to 0 and can't be changed. Hence, the docker build fails. The issue
> should be seen even if run as non-root.
> 3.) Next, as root, as per README, I ran: -
> server:/kafka> *bash tests/docker/run_tests.sh*
> The ducker tool builds the container images & switches to user '*ducker*'
> inside the container
> & maps kafka root dir ('kafka') from host to '/opt/kafka-dev' in the
> container.
> Ref:
> [https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak|https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak]
> Ex: docker run -d *-v "${kafka_dir}:/opt/kafka-dev"*
> This fails as the 'ducker' user has *no write permissions* to create files
> under 'kafka' root dir. Hence, it needs to be made writeable.
> // *chmod -R a+w kafka*
> – needed as container is run as 'ducker' and needs write access since kafka
> root volume from host is mapped to container as "/opt/kafka-dev" where the
> 'ducker' user writes logs
> =
> =
> *FIXES needed*
> ~
> 1.) Dockerfile -
> [https://github.com/apache/kafka/blob/trunk/tests/docker/Dockerfile]
> Change 'UID' to '*UID_DUCKER*'.
> This won't conflict with built in bash env. var UID and the docker image
> build should succeed.
> ---
> ARG *UID_DUCKER*="1000"
> RUN useradd -u $*UID_DUCKER* ducker
> // *{color:#57d9a3}No Error{color}* => No conflict with built-in UID
> ---
> 2.) README needs an update where we must ensure the kafka root dir from where
> the tests
> are launched is writeable to allow the 'ducker' user to create results/logs.
> # chmod -R a+w kafka
> With this, I was able to get the docker images built and system tests started
> successfully.
> =
> Also, I wonder whether or not upstream Dockerfile & System tests are part of
> CI/CD and get tested for every PR. If so, this issue should have been caught.
>
> *Question to kafka SME*
> -
> Do you believe this is a valid problem with the Dockerfile and the fix is
> acceptable?
> Please let me know and I am happy to submit a PR with this fix.
> Thanks,
> Abhijit
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-12847) Dockerfile needed for kafka system tests needs changes
[
https://issues.apache.org/jira/browse/KAFKA-12847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355791#comment-17355791
]
Abhijit Mane edited comment on KAFKA-12847 at 6/2/21, 3:12 PM:
---
[~chia7712]
Thanks for looking into this.
Dockerfile has an UID entry as shown below: -
---
ARG UID="1000" ==> Has no effect as UID value is read-only in bash
RUN useradd -u $UID ducker
// Error during docker build => useradd: UID 0 is not unique, root user id is 0
---
The issue is seen even if run as non-root. 'UID' is a built-in, read-only bash
environment variable and resolves to id of the logged in user.
PR submitted - https://github.com/apache/kafka/pull/10782
Please let me know your thoughts.
was (Author: abhijmanrh):
[~chia7712]
Thanks for looking into this.
Dockerfile has an UID entry as shown below: -
---
ARG UID="1000" ==> Has no effect as UID value is read-only in bash
RUN useradd -u $UID ducker
// Error during docker build => useradd: UID 0 is not unique, root user id is 0
---
The issue is seen even if run as non-root. 'UID' is a built-in, read-only bash
environment variable and resolves to id of the logged
in user. PR submitted - https://github.com/apache/kafka/pull/10782
Please let me know your thoughts.
> Dockerfile needed for kafka system tests needs changes
> --
>
> Key: KAFKA-12847
> URL: https://issues.apache.org/jira/browse/KAFKA-12847
> Project: Kafka
> Issue Type: Bug
> Components: system tests
>Affects Versions: 2.8.0, 2.7.1
> Environment: Issue tested in environments below but is independent of
> h/w arch. or Linux flavor: -
> 1.) RHEL-8.3 on x86_64
> 2.) RHEL-8.3 on IBM Power (ppc64le)
> 3.) apache/kafka branch tested: trunk (master)
>Reporter: Abhijit Mane
>Assignee: Abhijit Mane
>Priority: Major
> Labels: easyfix
> Attachments: Dockerfile.upstream
>
>
> Hello,
> I tried apache/kafka system tests as per documentation: -
> ([https://github.com/apache/kafka/tree/trunk/tests#readme|https://github.com/apache/kafka/tree/trunk/tests#readme_])
> =
> PROBLEM
> ~~
> 1.) As root user, clone kafka github repo and start "kafka system tests"
> # git clone [https://github.com/apache/kafka.git]
> # cd kafka
> # ./gradlew clean systemTestLibs
> # bash tests/docker/run_tests.sh
> 2.) Dockerfile issue -
> [https://github.com/apache/kafka/blob/trunk/tests/docker/Dockerfile]
> This file has an *UID* entry as shown below: -
> ---
> ARG *UID*="1000"
> RUN useradd -u $*UID* ducker
> // {color:#de350b}*Error during docker build*{color} => useradd: UID 0 is not
> unique, root user id is 0
> ---
> I ran everything as root which means the built-in bash environment variable
> 'UID' always
> resolves to 0 and can't be changed. Hence, the docker build fails. The issue
> should be seen even if run as non-root.
> 3.) Next, as root, as per README, I ran: -
> server:/kafka> *bash tests/docker/run_tests.sh*
> The ducker tool builds the container images & switches to user '*ducker*'
> inside the container
> & maps kafka root dir ('kafka') from host to '/opt/kafka-dev' in the
> container.
> Ref:
> [https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak|https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak]
> Ex: docker run -d *-v "${kafka_dir}:/opt/kafka-dev"*
> This fails as the 'ducker' user has *no write permissions* to create files
> under 'kafka' root dir. Hence, it needs to be made writeable.
> // *chmod -R a+w kafka*
> – needed as container is run as 'ducker' and needs write access since kafka
> root volume from host is mapped to container as "/opt/kafka-dev" where the
> 'ducker' user writes logs
> =
> =
> *FIXES needed*
> ~
> 1.) Dockerfile -
> [https://github.com/apache/kafka/blob/trunk/tests/docker/Dockerfile]
> Change 'UID' to '*UID_DUCKER*'.
> This won't conflict with built in bash env. var UID and the docker image
> build should succeed.
> ---
> ARG *UID_DUCKER*="1000"
> RUN useradd -u $*UID_DUCKER* ducker
> // *{color:#57d9a3}No Error{color}* => No conflict with built-in UID
> ---
> 2.) README needs an update where we must ensure the kafka root dir from where
> the tests
> are launched is writeable to allow the 'ducker' user to create results/logs.
> # chmod -R a+w kafka
> With this, I was able to get the docker images built and system tests started
> successfully.
> =
> Also, I wonder whether or not upstream Dockerfile & System tests are part of
> CI/CD
[jira] [Commented] (KAFKA-12847) Dockerfile needed for kafka system tests needs changes
[
https://issues.apache.org/jira/browse/KAFKA-12847?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355791#comment-17355791
]
Abhijit Mane commented on KAFKA-12847:
--
[~chia7712]
Thanks for looking into this.
Dockerfile has an UID entry as shown below: -
---
ARG UID="1000" ==> Has no effect as UID value is read-only in bash
RUN useradd -u $UID ducker
// Error during docker build => useradd: UID 0 is not unique, root user id is 0
---
The issue is seen even if run as non-root. 'UID' is a built-in, read-only bash
environment variable and resolves to id of the logged
in user. PR submitted - https://github.com/apache/kafka/pull/10782
Please let me know your thoughts.
> Dockerfile needed for kafka system tests needs changes
> --
>
> Key: KAFKA-12847
> URL: https://issues.apache.org/jira/browse/KAFKA-12847
> Project: Kafka
> Issue Type: Bug
> Components: system tests
>Affects Versions: 2.8.0, 2.7.1
> Environment: Issue tested in environments below but is independent of
> h/w arch. or Linux flavor: -
> 1.) RHEL-8.3 on x86_64
> 2.) RHEL-8.3 on IBM Power (ppc64le)
> 3.) apache/kafka branch tested: trunk (master)
>Reporter: Abhijit Mane
>Assignee: Abhijit Mane
>Priority: Major
> Labels: easyfix
> Attachments: Dockerfile.upstream
>
>
> Hello,
> I tried apache/kafka system tests as per documentation: -
> ([https://github.com/apache/kafka/tree/trunk/tests#readme|https://github.com/apache/kafka/tree/trunk/tests#readme_])
> =
> PROBLEM
> ~~
> 1.) As root user, clone kafka github repo and start "kafka system tests"
> # git clone [https://github.com/apache/kafka.git]
> # cd kafka
> # ./gradlew clean systemTestLibs
> # bash tests/docker/run_tests.sh
> 2.) Dockerfile issue -
> [https://github.com/apache/kafka/blob/trunk/tests/docker/Dockerfile]
> This file has an *UID* entry as shown below: -
> ---
> ARG *UID*="1000"
> RUN useradd -u $*UID* ducker
> // {color:#de350b}*Error during docker build*{color} => useradd: UID 0 is not
> unique, root user id is 0
> ---
> I ran everything as root which means the built-in bash environment variable
> 'UID' always
> resolves to 0 and can't be changed. Hence, the docker build fails. The issue
> should be seen even if run as non-root.
> 3.) Next, as root, as per README, I ran: -
> server:/kafka> *bash tests/docker/run_tests.sh*
> The ducker tool builds the container images & switches to user '*ducker*'
> inside the container
> & maps kafka root dir ('kafka') from host to '/opt/kafka-dev' in the
> container.
> Ref:
> [https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak|https://github.com/apache/kafka/blob/trunk/tests/docker/ducker-ak]
> Ex: docker run -d *-v "${kafka_dir}:/opt/kafka-dev"*
> This fails as the 'ducker' user has *no write permissions* to create files
> under 'kafka' root dir. Hence, it needs to be made writeable.
> // *chmod -R a+w kafka*
> – needed as container is run as 'ducker' and needs write access since kafka
> root volume from host is mapped to container as "/opt/kafka-dev" where the
> 'ducker' user writes logs
> =
> =
> *FIXES needed*
> ~
> 1.) Dockerfile -
> [https://github.com/apache/kafka/blob/trunk/tests/docker/Dockerfile]
> Change 'UID' to '*UID_DUCKER*'.
> This won't conflict with built in bash env. var UID and the docker image
> build should succeed.
> ---
> ARG *UID_DUCKER*="1000"
> RUN useradd -u $*UID_DUCKER* ducker
> // *{color:#57d9a3}No Error{color}* => No conflict with built-in UID
> ---
> 2.) README needs an update where we must ensure the kafka root dir from where
> the tests
> are launched is writeable to allow the 'ducker' user to create results/logs.
> # chmod -R a+w kafka
> With this, I was able to get the docker images built and system tests started
> successfully.
> =
> Also, I wonder whether or not upstream Dockerfile & System tests are part of
> CI/CD and get tested for every PR. If so, this issue should have been caught.
>
> *Question to kafka SME*
> -
> Do you believe this is a valid problem with the Dockerfile and the fix is
> acceptable?
> Please let me know and I am happy to submit a PR with this fix.
> Thanks,
> Abhijit
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-10900) Add metrics enumerated in KIP-630
[ https://issues.apache.org/jira/browse/KAFKA-10900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355728#comment-17355728 ] loboxu edited comment on KAFKA-10900 at 6/2/21, 3:11 PM: - [~jagsancio] Can you give me some guidance? Where the metrics are added in the code. I have looked at the code and the logic is not very clear. Here is my understanding of the position: * GenSnapshotLatencyMs：KafkaMetadataLog.createSnapshot() - LoadSnapshotLatencyMs：KafkaMetadataLog.readSnapshot() - SnapshotSizeBytes: KafkaMetadataLog.createSnapshot() - SnapshotLag: KafkaMetadataLog.appendAsLeader() was (Author: loboxu): [~jagsancio] Can you give me some guidance? Show me where these four metrics are added in the code. I have looked at the code and the logic is not very clear. Here is my understanding of the position: * GenSnapshotLatencyMs：KafkaMetadataLog.createSnapshot() - LoadSnapshotLatencyMs：KafkaMetadataLog.readSnapshot() - SnapshotSizeBytes - SnapshotLag > Add metrics enumerated in KIP-630 > - > > Key: KAFKA-10900 > URL: https://issues.apache.org/jira/browse/KAFKA-10900 > Project: Kafka > Issue Type: Sub-task > Components: replication >Reporter: Jose Armando Garcia Sancio >Assignee: loboxu >Priority: Major > > KIP-630 enumerates a few metrics. Makes sure that those metrics are > implemented. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-10900) Add metrics enumerated in KIP-630
[ https://issues.apache.org/jira/browse/KAFKA-10900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355728#comment-17355728 ] loboxu edited comment on KAFKA-10900 at 6/2/21, 3:04 PM: - [~jagsancio] Can you give me some guidance? Show me where these four metrics are added in the code. I have looked at the code and the logic is not very clear. Here is my understanding of the position: * GenSnapshotLatencyMs：KafkaMetadataLog.createSnapshot() - LoadSnapshotLatencyMs：KafkaMetadataLog.readSnapshot() - SnapshotSizeBytes - SnapshotLag was (Author: loboxu): [~jagsancio] Can you give me some guidance? Show me where these four metrics are added in the code. - GenSnapshotLatencyMs - LoadSnapshotLatencyMs - SnapshotSizeBytes - SnapshotLag I have looked at the code and the logic is not very clear. > Add metrics enumerated in KIP-630 > - > > Key: KAFKA-10900 > URL: https://issues.apache.org/jira/browse/KAFKA-10900 > Project: Kafka > Issue Type: Sub-task > Components: replication >Reporter: Jose Armando Garcia Sancio >Assignee: loboxu >Priority: Major > > KIP-630 enumerates a few metrics. Makes sure that those metrics are > implemented. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (KAFKA-10900) Add metrics enumerated in KIP-630
[ https://issues.apache.org/jira/browse/KAFKA-10900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355728#comment-17355728 ] loboxu commented on KAFKA-10900: [~jagsancio] Can you give me some guidance? Show me where these four metrics are added in the code. - GenSnapshotLatencyMs - LoadSnapshotLatencyMs - SnapshotSizeBytes - SnapshotLag I have looked at the code and the logic is not very clear. > Add metrics enumerated in KIP-630 > - > > Key: KAFKA-10900 > URL: https://issues.apache.org/jira/browse/KAFKA-10900 > Project: Kafka > Issue Type: Sub-task > Components: replication >Reporter: Jose Armando Garcia Sancio >Assignee: loboxu >Priority: Major > > KIP-630 enumerates a few metrics. Makes sure that those metrics are > implemented. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] ijuma commented on pull request #10808: KAFKA-12880: Remove deprecated `Count` and `SampledTotal` in 3.0
ijuma commented on pull request #10808: URL: https://github.com/apache/kafka/pull/10808#issuecomment-853032815 @dajac did you intend to keep these when you removed `Sum` and `Total` or was it an oversight? -- 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 #10808: KAFKA-12880: Remove deprecated `Count` and `SampledTotal` in 3.0
ijuma opened a new pull request #10808: URL: https://github.com/apache/kafka/pull/10808 They were both deprecated in Apache Kafka 2.4 and it's a straightforward change to use the non deprecated variants. ### 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] [Updated] (KAFKA-12880) Remove deprecated Count and SampledTotal in 3.0
[ https://issues.apache.org/jira/browse/KAFKA-12880?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ismael Juma updated KAFKA-12880: Summary: Remove deprecated Count and SampledTotal in 3.0 (was: Remove deprecated Count and Rate in 3.0) > Remove deprecated Count and SampledTotal in 3.0 > --- > > Key: KAFKA-12880 > URL: https://issues.apache.org/jira/browse/KAFKA-12880 > Project: Kafka > Issue Type: Bug >Reporter: Ismael Juma >Assignee: Ismael Juma >Priority: Major > Fix For: 3.0.0 > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (KAFKA-12880) Remove deprecated Count and Rate in 3.0
Ismael Juma created KAFKA-12880: --- Summary: Remove deprecated Count and Rate in 3.0 Key: KAFKA-12880 URL: https://issues.apache.org/jira/browse/KAFKA-12880 Project: Kafka Issue Type: Bug Reporter: Ismael Juma Assignee: Ismael Juma Fix For: 3.0.0 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] ijuma merged pull request #10806: MINOR: update kafka-topics.sh line command tool upgrade notes with removed option
ijuma merged pull request #10806: URL: https://github.com/apache/kafka/pull/10806 -- 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 merged pull request #10800: MINOR: Update jmh for async profiler 2.0 support
ijuma merged pull request #10800: URL: https://github.com/apache/kafka/pull/10800 -- 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-12598) Remove deprecated --zookeeper in ConfigCommand
[ https://issues.apache.org/jira/browse/KAFKA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355675#comment-17355675 ] Luke Chen commented on KAFKA-12598: --- [~ijuma], I see. Will do! Thanks. > Remove deprecated --zookeeper in ConfigCommand > -- > > Key: KAFKA-12598 > URL: https://issues.apache.org/jira/browse/KAFKA-12598 > Project: Kafka > Issue Type: Sub-task >Reporter: Luke Chen >Assignee: Luke Chen >Priority: Critical > Fix For: 3.0.0 > > -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [kafka] tombentley commented on pull request #10807: KAFKA-12797: Log the evictor of fetch sessions
tombentley commented on pull request #10807: URL: https://github.com/apache/kafka/pull/10807#issuecomment-852970504 @dajac please could you review? -- 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] tombentley opened a new pull request #10807: KAFKA-12797: Log the evictor of fetch sessions
tombentley opened a new pull request #10807: URL: https://github.com/apache/kafka/pull/10807 [KAFKA-12797](https://issues.apache.org/jira/browse/KAFKA-12797) describes the difficulty in discovering bad clients that are responsible for evicting every other non-privileged session from the cache. While the ideal solution would be to have a quota (so a user ends up evicting only their own sessions once quota was reached), the fetch session cache is very performance sensitive, and this problem is very rare, so logging seems like a reasonable first step. -- 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] feyman2016 commented on pull request #10593: KAFKA-10800 Validate the snapshot id when the state machine creates a snapshot
feyman2016 commented on pull request #10593: URL: https://github.com/apache/kafka/pull/10593#issuecomment-852954993 @mumrah Thanks for the feedback, just addressed, but the potential concurrency issue needs more discussion~ -- 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] feyman2016 commented on pull request #10593: KAFKA-10800 Validate the snapshot id when the state machine creates a snapshot
feyman2016 commented on pull request #10593: URL: https://github.com/apache/kafka/pull/10593#issuecomment-852954180 > > Are there any concerns with multi-threading here? I think the answer is "no". Any thoughts @feyman2016 or @jsancio? > > Good point. There may be some concurrency issue with `createSnapshot`. How about moving the validation to `onSnapshotFrozen`? > > This conversation also reminded me to create this issue: https://issues.apache.org/jira/browse/KAFKA-12873. It is slightly related to this point. @jsancio Can you share more details about the possible concurrency scenario with `createSnapshot `? BTW, will moving the validation to `onSnapshotFrozen` imply that before creating the snapshot, there's no validation? I think maybe we can keep the validation here and add some additional check before `freeze()` which makes the snapshot visible? -- 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] feyman2016 commented on a change in pull request #10593: KAFKA-10800 Validate the snapshot id when the state machine creates a snapshot
feyman2016 commented on a change in pull request #10593:
URL: https://github.com/apache/kafka/pull/10593#discussion_r643850904
##
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##
@@ -2276,6 +2277,24 @@ public void resign(int epoch) {
);
}
+private void validateSnapshotId(OffsetAndEpoch snapshotId) {
+Optional highWatermarkOpt =
quorum().highWatermark();
+if (!highWatermarkOpt.isPresent() || highWatermarkOpt.get().offset <
snapshotId.offset) {
+throw new IllegalArgumentException("Trying to creating snapshot
with invalid snapshotId: " + snapshotId + " whose offset is larger than the
high-watermark: " +
Review comment:
Fixed~
--
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] feyman2016 commented on a change in pull request #10593: KAFKA-10800 Validate the snapshot id when the state machine creates a snapshot
feyman2016 commented on a change in pull request #10593:
URL: https://github.com/apache/kafka/pull/10593#discussion_r643850801
##
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##
@@ -2276,6 +2277,24 @@ public void resign(int epoch) {
);
}
+private void validateSnapshotId(OffsetAndEpoch snapshotId) {
Review comment:
Make sense, fixed
--
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 #10806: MINOR: update kafka-topics.sh line command tool upgrade notes with removed option
wenbingshen commented on pull request #10806: URL: https://github.com/apache/kafka/pull/10806#issuecomment-852907017 related to #10457 . Please take a look. @showuon @ijuma -- 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 opened a new pull request #10806: MINOR: update kafka-topics.sh line command tool upgrade notes with removed option
wenbingshen opened a new pull request #10806: URL: https://github.com/apache/kafka/pull/10806 As the title. ### 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] showuon commented on pull request #10457: KAFKA-12596: remove --zookeeper option from topic command
showuon commented on pull request #10457: URL: https://github.com/apache/kafka/pull/10457#issuecomment-852894925 Will address the above 2 comments in #10471. Thank you. -- 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] dajac merged pull request #10801: MINOR: remove unneccessary public keyword from ProducerInterceptor/ConsumerInterceptor interface
dajac merged pull request #10801: URL: https://github.com/apache/kafka/pull/10801 -- 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-12879) Compatibility break in Admin.listOffsets()
Tom Bentley created KAFKA-12879:
---
Summary: Compatibility break in Admin.listOffsets()
Key: KAFKA-12879
URL: https://issues.apache.org/jira/browse/KAFKA-12879
Project: Kafka
Issue Type: Bug
Components: admin
Affects Versions: 2.6.2, 2.7.1, 2.8.0
Reporter: Tom Bentley
KAFKA-12339 incompatibly changed the semantics of Admin.listOffsets().
Previously it would fail with {{UnknownTopicOrPartitionException}} when a topic
didn't exist. Now it will (eventually) fail with {{TimeoutException}}. It seems
this was more or less intentional, even though it would break code which was
expecting and handling the {{UnknownTopicOrPartitionException}}. A workaround
is to use {{retries=1}} and inspect the cause of the {{TimeoutException}}, but
this isn't really suitable for cases where the same Admin client instance is
being used for other calls where retries is desirable.
Furthermore as well as the intended effect on {{listOffsets()}} it seems that
the change could actually affect other methods of Admin.
More generally, the Admin client API is vague about which exceptions can
propagate from which methods. This means that it's not possible to say, in
cases like this, whether the calling code _should_ have been relying on the
{{UnknownTopicOrPartitionException}} or not.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-10681) MM2 translateOffsets returns wrong offsets
[
https://issues.apache.org/jira/browse/KAFKA-10681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17355371#comment-17355371
]
Sandeep mehta edited comment on KAFKA-10681 at 6/2/21, 8:42 AM:
I am not sure why its happening with *RemoteClusterUtils.translateOffsets().*
But it's not required anymore with the latest versions of mm2. You can use
Mirrormaker2 from Kafka 2.7.0. It comes with automated consumer offset sync,
which translates consumer offsets automatically.
We have also used Strimzi (with Kafka 2.7.0) for our Kafka clusters 2.6.0.
https://issues.apache.org/jira/browse/KAFKA-9076
I hope it helps.
was (Author: sandeep26nov):
I am not sure why its happening with *RemoteClusterUtils.translateOffsets().*
But it's not required anymore with the latest versions of mm2. You can use
Mirrormaker2 from Kafka 2.7.0. It comes with automated consumer offset sync,
which translates consumer offsets automatically.
https://issues.apache.org/jira/browse/KAFKA-9076
I hope it helps
> MM2 translateOffsets returns wrong offsets
> --
>
> Key: KAFKA-10681
> URL: https://issues.apache.org/jira/browse/KAFKA-10681
> Project: Kafka
> Issue Type: Bug
> Components: mirrormaker
>Affects Versions: 2.5.0
> Environment: GKE, strimzi release
>Reporter: Carlo Bongiovanni
>Priority: Major
>
> Hi all,
> we'd like to make use of the ability of MM2 to mirror checkpoints of consumer
> offsets, in order to have a graceful failover from an active cluster to a
> standby one.
> For this reason we have created the following setup (FYI all done with
> strimzi on k8s):
> * an active kafka cluster 2.5.0 used by a few producers/consumers
> * a standby kafka cluster 2.5.0
> * MM2 is setup in one direction only to mirror from active to standby
> We have let MM2 run for some time and we could verify that messages are
> effectively mirrored.
> At this point we have started developing the tooling to create consumer
> groups in the consumer-offsets topic of the passive cluster, by reading the
> internal checkpoints topic.
> The following is an extract of our code to read the translated offsets:
> {code:java}
> Map mm2Props = new HashMap<>();
> mm2Props.put(BOOTSTRAP_SERVERS_CONFIG, "bootstrap_servers");
> mm2Props.put("source.cluster.alias", "euwe");
> mm2Props.put(SASL_MECHANISM, "SCRAM-SHA-512");
> mm2Props.put(SASL_JAAS_CONFIG,
> "org.apache.kafka.common.security.scram.ScramLoginModule required
> username=\"user\" password=\"password\";");
> mm2Props.put(SECURITY_PROTOCOL_CONFIG, "SASL_SSL");
> mm2Props.put(SSL_TRUSTSTORE_LOCATION_CONFIG,
> "/usr/local/lib/jdk/lib/security/cacerts");
> mm2Props.put(SSL_TRUSTSTORE_PASSWORD_CONFIG, "some-password");
> Map translatedOffsets = RemoteClusterUtils
> .translateOffsets(mm2Props, (String) mm2Props.get("source.cluster.alias"),
> cgi,
> Duration.ofSeconds(60L));
> {code}
>
> Before persisting the translated offsets with
> {code:java}
> AlterConsumerGroupOffsetsResult alterConsumerGroupOffsetsResult = kafkaClient
> .alterConsumerGroupOffsets(cgi, offsets);{code}
> we filter them because we don't want to create consumer groups for all
> retrieved offsets.
> During the filtering, we compare the values of the translated offset for each
> topic partition (as coming from the checkpoint topic),
> with the respective current offset value for each topic partition (as
> mirrored from MM2).
> While running this check we have verified that for some topics we get big
> difference between those values, while for other topics the update seems
> realistic.
> For example, looking at a given target partition we see it has an offset of
> 100 (after mirroring by mm2).
> From the checkpoint topic for the same consumer group id, we receive offset
> 200, and later 150.
> The issues are that:
> * both consumer group id offsets exceed the real offset of the partition
> * the consumer group id offsets from checkpoint goes down over time, not up
> We haven't been able to explain it, the wrong numbers are coming from the
> *RemoteClusterUtils.translateOffsets()* and we're wondering if this could be
> a misconfiguration on our side or a bug of MM2.
> Thanks, best
> C.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [kafka] showuon commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
showuon commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r643754484
##
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##
@@ -58,7 +58,11 @@ import scala.util.{Failure, Success, Try}
*enable.auto.commit=false
* 3. Mirror Maker Setting:
*abort.on.send.failure=true
+ *
+ * @deprecated The original Mirror Maker is deprecated since release 3.0.
Similar functionality can be
+ *found in the Connect-based re-implementation by the same name (aka MM2).
Review comment:
1. `@deprecated The original Mirror Maker is deprecated since release
3.0.` -> could we just say `@deprecated since release 3.0.` ?
2. `Similar functionality can be found in the Connect-based
re-implementation by the same name (aka MM2)` -> Could we change to: `Please
use Connect-based re-implementation in
org.apache.kafka.connect.mirror.MirrorMaker (aka MM2) instead.`?
--
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] showuon commented on a change in pull request #10805: KAFKA-12436 KIP-720 Deprecate MirrorMaker v1
showuon commented on a change in pull request #10805:
URL: https://github.com/apache/kafka/pull/10805#discussion_r643754484
##
File path: core/src/main/scala/kafka/tools/MirrorMaker.scala
##
@@ -58,7 +58,11 @@ import scala.util.{Failure, Success, Try}
*enable.auto.commit=false
* 3. Mirror Maker Setting:
*abort.on.send.failure=true
+ *
+ * @deprecated The original Mirror Maker is deprecated since release 3.0.
Similar functionality can be
+ *found in the Connect-based re-implementation by the same name (aka MM2).
Review comment:
1. `The original Mirror Maker` -> could we say `The old Mirror Maker` ?
2. `Similar functionality can be found in the Connect-based
re-implementation by the same name (aka MM2)` -> Could we change to: `Please
use Connect-based re-implementation in
org.apache.kafka.connect.mirror.MirrorMaker (aka MM2) instead.`?
--
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
