Skip to content

KAFKA-9793: Expand the try-catch for task commit in HandleAssignment#8402

Merged
guozhangwang merged 2 commits intoapache:trunkfrom
abbccdda:KAFKA-9793
Apr 6, 2020
Merged

KAFKA-9793: Expand the try-catch for task commit in HandleAssignment#8402
guozhangwang merged 2 commits intoapache:trunkfrom
abbccdda:KAFKA-9793

Conversation

@abbccdda
Copy link
Contributor

@abbccdda abbccdda commented Apr 1, 2020

As title suggests, we would like to broaden this check so that we don't fail to close a doom-to-cleanup task.

Committer Checklist (excluded from commit message)

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

@abbccdda abbccdda force-pushed the KAFKA-9793 branch 3 times, most recently from bbe55d8 to 76be91c Compare April 1, 2020 06:49
@guozhangwang
Copy link
Contributor

test this please

@mjsax
Copy link
Member

mjsax commented Apr 2, 2020

Java 11 passed, but Java 8 failed with

java.lang.AssertionError: unexpected exception type thrown; expected:<java.lang.RuntimeException> but was:<java.lang.AssertionError>
	at org.junit.Assert.assertThrows(Assert.java:1020)
	at org.junit.Assert.assertThrows(Assert.java:981)
	at org.apache.kafka.streams.processor.internals.TaskManagerTest.shouldCloseActiveTasksDirtyAndPropagatePrepareCommitException(TaskManagerTest.java:1270)

Could this be related?

@abbccdda
Copy link
Contributor Author

abbccdda commented Apr 2, 2020

Let me check @mjsax

@abbccdda
Copy link
Contributor Author

abbccdda commented Apr 2, 2020

Fixed by relaxing the ordering check @mjsax

@mjsax
Copy link
Member

mjsax commented Apr 2, 2020

Retest this please.

@abbccdda
Copy link
Contributor Author

abbccdda commented Apr 3, 2020

Retest this please

@guozhangwang
Copy link
Contributor

test this please

@abbccdda
Copy link
Contributor Author

abbccdda commented Apr 3, 2020

Only one test failure on jdk 8:

kafka.api.PlaintextAdminIntegrationTest.testAlterReplicaLogDirs

Error Message
org.scalatest.exceptions.TestFailedException: only 0 messages are produced within timeout after replica movement. Producer future Some(Failure(java.util.concurrent.TimeoutException: Timeout after waiting for 10000 ms.))

@mjsax
Copy link
Member

mjsax commented Apr 4, 2020

Retest this please.

@guozhangwang
Copy link
Contributor

10:11:30 Execution failed for task ':streams:compileJava'.
10:11:30 > Compilation failed; see the compiler error output for details.

@abbccdda
Copy link
Contributor Author

abbccdda commented Apr 4, 2020

@guozhangwang I think we need to merge #8424 first

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, after fixing jenkins this should be good to go.

@guozhangwang
Copy link
Contributor

test this please

2 similar comments
@guozhangwang
Copy link
Contributor

test this please

@guozhangwang
Copy link
Contributor

test this please

@guozhangwang
Copy link
Contributor

test this

@guozhangwang guozhangwang reopened this Apr 5, 2020
@guozhangwang
Copy link
Contributor

test this please

@guozhangwang
Copy link
Contributor

Okay I'm done with jenkins bot and manually build two: https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1622 and https://builds.apache.org/job/kafka-pr-jdk11-scala2.13/5641

@guozhangwang
Copy link
Contributor

Both jenkins succeed, merging to trunk now.

@guozhangwang guozhangwang merged commit 712ac52 into apache:trunk Apr 6, 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.

3 participants