Skip to content

KAFKA-9610: Do not throw illegal state when remaining partitions are not empty#8169

Merged
guozhangwang merged 5 commits intoapache:trunkfrom
abbccdda:no_illegal_state
Feb 26, 2020
Merged

KAFKA-9610: Do not throw illegal state when remaining partitions are not empty#8169
guozhangwang merged 5 commits intoapache:trunkfrom
abbccdda:no_illegal_state

Conversation

@abbccdda
Copy link
Contributor

For handleRevocation, it is possible that previous onAssignment callback has cleaned up the stream tasks, which means no corresponding task could be found for given partitions. We should not throw here as this is expected behavior.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really make sense, onPartitionsRevoked is always called before onAssignment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want to log as warn or debug though -- hitting this indicates we hit the (likely not that common) race condition, or a bug.

@abbccdda
Copy link
Contributor Author

@guozhangwang could you also take a look and see if you could trigger a test?

@guozhangwang
Copy link
Contributor

test this please

Copy link
Contributor

@guozhangwang guozhangwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@abbccdda
Copy link
Contributor Author

abbccdda commented Feb 26, 2020

The failure was really weird as it is not related to the change:

20:20:40 > Task :streams:compileTestJava
20:20:40 /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13/streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java:351: error: method getAllTopicsInCluster in class KafkaZkClient cannot be applied to given types;
20:20:40         return JavaConverters.setAsJavaSetConverter(brokers[0].kafkaServer().zkClient().getAllTopicsInCluster()).asJava();
20:20:40                                                                                        ^
20:20:40   required: boolean
20:20:40   found: no arguments
20:20:40   reason: actual and formal argument lists differ in length
20:20:49 1 error

Will rebase trunk and see if this fixes

@abbccdda
Copy link
Contributor Author

@omkreddy
Copy link
Contributor

The failure was really weird as it is not related to the change:

20:20:40 > Task :streams:compileTestJava
20:20:40 /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13/streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java:351: error: method getAllTopicsInCluster in class KafkaZkClient cannot be applied to given types;
20:20:40         return JavaConverters.setAsJavaSetConverter(brokers[0].kafkaServer().zkClient().getAllTopicsInCluster()).asJava();
20:20:40                                                                                        ^
20:20:40   required: boolean
20:20:40   found: no arguments
20:20:40   reason: actual and formal argument lists differ in length
20:20:49 1 error

Will rebase trunk and see if this fixes

Fixed in #8170

@guozhangwang
Copy link
Contributor

test this please

1 similar comment
@guozhangwang
Copy link
Contributor

test this please

@guozhangwang guozhangwang merged commit a983545 into apache:trunk Feb 26, 2020
@abbccdda abbccdda changed the title KAFKA-9610: do not throw illegal state when remaining partitions are not empty KAFKA-9610: Do not throw illegal state when remaining partitions are not empty Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants