Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINOR: Fix bug in waitUntilLeaderIsElectedOrChanged and simplify result type #3031

Closed
wants to merge 1 commit into from

Conversation

ijuma
Copy link
Contributor

@ijuma ijuma commented May 12, 2017

Also disable a couple of tests that were passing incorrectly until KAFKA-3096 is fixed.

The bug was for the following case:

leader.isDefined && oldLeaderOpt.isEmpty && newLeaderOpt.isDefined && newLeaderOpt.get != leader.get

We would consider it a successful election even though the new leader was not the expected leader.

I also changed the result type as we never return None (we throw an exception instead).

@ijuma ijuma force-pushed the fix-wait-until-leader-is-elected branch from 75a4410 to d2a1d62 Compare May 12, 2017 09:33
…sult type

Also disable a couple of tests that were passing incorrectly until KAFKA-3096 is fixed.

The bug was for the following case:

`leader.isDefined && oldLeaderOpt.isEmpty && newLeaderOpt.isDefined && newLeaderOpt.get != leader.get`

We would consider it a successful election even though we should not.

I also changed the result type as we never return `None` (we throw an exception instead).
@ijuma ijuma force-pushed the fix-wait-until-leader-is-elected branch from d2a1d62 to 792fc2a Compare May 12, 2017 10:35
@asfbot
Copy link

asfbot commented May 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/3804/
Test FAILed (JDK 7 and Scala 2.11).

@asfbot
Copy link

asfbot commented May 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/3793/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link

asfbot commented May 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/3812/
Test PASSed (JDK 7 and Scala 2.11).

@asfbot
Copy link

asfbot commented May 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/3800/
Test PASSed (JDK 8 and Scala 2.12).

@ijuma
Copy link
Contributor Author

ijuma commented May 13, 2017

Review by @junrao

@junrao
Copy link
Contributor

junrao commented May 15, 2017

@ijuma : Thanks for the patch. Good find and a great cleanup patch. LGTM.

@asfgit asfgit closed this in dd9f431 May 15, 2017
@ijuma ijuma deleted the fix-wait-until-leader-is-elected branch June 18, 2017 09:33
andrewegel pushed a commit to confluentinc/kafka that referenced this pull request Mar 10, 2021
* Replace asynchronous mocked calls with synchronous mocks
Signed-off-by: Greg Harris <gregh@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants