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

Shutdown the status thread as soon as clients complete (Issue #316) #359

Merged
merged 3 commits into from
Jul 18, 2015

Conversation

allanbank
Copy link
Collaborator

There are two commits.

The first has the actual fix which uses a CountDownLatch to notify the StatusThread when all of the clients have completed.

The second removes whitespace at the ends of lines and replaces tabs with 4 spaces.

@bigbes
Copy link
Collaborator

bigbes commented Jul 16, 2015

alldone = _completeLatch.await(deadline-now, TimeUnit.NANOSECONDS);
}
catch( InterruptedException ie) {
// Handled by while loop.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we get interrupted, we ought to return false instead of continuing to wait. (Presuming that will cause us to exit soon)

Should also set the thread's interrupted status.

@busbey
Copy link
Collaborator

busbey commented Jul 16, 2015

Good catch @bigbes!

One other small nit, please update the commit messages to start with what they impact, in this case "[client]".

@allanbank allanbank force-pushed the pr-316 branch 2 times, most recently from 65a6a3b to e20bcbd Compare July 17, 2015 00:56
@allanbank
Copy link
Collaborator Author

I amended the commits.

@allanbank
Copy link
Collaborator Author

I merged in pr-#286's patch since the two pr's conflict.

@busbey
Copy link
Collaborator

busbey commented Jul 18, 2015

can you amend the last commit to keep my authorship?

So long as it's the HEAD commit the command loolks like

 git commit --amend --author "Sean Busbey <busbey@apache.org>"

It'll open the commit message in your editor, just save and quit. You'll still show up as the committer when thing are in the repo. You'll have to force push the branch to update.

@allanbank
Copy link
Collaborator Author

allanbank commented Jul 18, 2015 via email

@busbey
Copy link
Collaborator

busbey commented Jul 18, 2015

👍

allanbank added a commit that referenced this pull request Jul 18, 2015
Shutdown the status thread as soon as clients complete (Issue #316)
@allanbank allanbank merged commit 4d270b6 into brianfrankcooper:master Jul 18, 2015
@allanbank allanbank deleted the pr-316 branch July 19, 2015 20:44
@busbey busbey mentioned this pull request Aug 17, 2015
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
Shutdown the status thread as soon as clients complete (Issue brianfrankcooper#316)
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
Shutdown the status thread as soon as clients complete (Issue brianfrankcooper#316)
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