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

STORM-3168 prevent AsyncLocalizer cleanup from crashing #2786

Merged
merged 1 commit into from Aug 3, 2018

Conversation

agresch
Copy link
Contributor

@agresch agresch commented Aug 1, 2018

Just catching and logging any error that may be occurring.

@srdo
Copy link
Contributor

srdo commented Aug 2, 2018

Seems reasonable to me, but do we really want to catch the Error subtypes as well?

@agresch
Copy link
Contributor Author

agresch commented Aug 2, 2018

If an error occurred, what should be the expected behavior? Shutdown?

@srdo
Copy link
Contributor

srdo commented Aug 2, 2018

Yes, we probably don't want to keep running if we hit an Error.

@agresch
Copy link
Contributor Author

agresch commented Aug 2, 2018

@srdo - updated to shutdown on errors. Filed https://issues.apache.org/jira/browse/STORM-3174 to standardize exit codes. I'm not clear on why we're selecting the values we are.

@srdo
Copy link
Contributor

srdo commented Aug 2, 2018

Thanks. +1

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

+1

@asfgit asfgit merged commit 5c6e102 into apache:master Aug 3, 2018
asfgit pushed a commit that referenced this pull request Aug 3, 2018
…/storm into STORM-3168

STORM-3168: prevent AsyncLocalizer cleanup from crashing

This closes #2786
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants