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

KAFKA-9903 #8535

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

KAFKA-9903 #8535

wants to merge 1 commit into from

Conversation

lushilin
Copy link

@lushilin lushilin commented Apr 22, 2020

ShutdownComplete will countdown in the finally block when thread shutdown due to an error, and in this case thread is not running.
So isRunning logic should check isShutdownInitiated and isShutdownComplete.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

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

ShutdownComplete will countdown in the finally block when thread shutdown due to an error, and in this case thread is not running.
So isRunning logic should check isShutdownInitiated and isShutdownComplete.
@lushilin
Copy link
Author

@guozhangwang @ijuma @junrao PTAL, thanks

@guozhangwang
Copy link
Contributor

test this please

@guozhangwang
Copy link
Contributor

@lushilin could we also augment the test under ShutdownableThreadTest to verify that isRunning is false and isThreadFailed is true before calling thread.shutdown and then becomes false after calling thread.shutdown.

@guozhangwang
Copy link
Contributor

test this please

@guozhangwang
Copy link
Contributor

Also could you run the unit test suite locally (you can see README.md for instructions), the jenkins failures seem relevant.

@lushilin
Copy link
Author

ok,i will write test case and run unit test locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants