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
[FLINK-1492] Fix exceptions on blob store shutdown #376
Conversation
This change is also in 0.8 so do we need to apply the fix there as well for the upcoming 0.8.1 release? |
Yes, if it is finally OK. |
Looks good. |
} | ||
} | ||
try { | ||
join(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this join do? I'm at a loss here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's from the old code and I am not sure if it really needs to stay, but it ensures that the BlobServer thread really finishes when calling the shutdown method (BlobServer is a Thread and because the join is called from outside of the run method it waits for the BlobServer thread to finish).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah of course. Thanks for the clarification.
LGTM except for my single question. |
I'll merge the change. |
OK, thanks. Just a reminder: we need to include this in 0.8.1 as well. |
I still get
On the last night's master. |
OK, thanks for reporting this. @StephanEwen has some pending changes to the blob manager and he will look into it as well. It is not allowed to remove the shutdown hook when a shutdown is already in progress. |
The problem is totally deterministic when shutting down the job/task managers. I've fixed it here: https://github.com/uce/incubator-flink/tree/flink-1492-fix_exceptions_really @StephanEwen, if this is blocking the release vote, we should just merge my change and not wait for your changes in master, which touch the blob store. |
No description provided.