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

Use a dedicated id to serialize EsExceptions instead of it's class name. #13629

Merged
merged 1 commit into from Sep 17, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Sep 17, 2015

Classnames change quickly due to refactorings etc. If that happens in a minor release
we loose the ability to deserialize the exceptoin coming from another node sicne we today
look it up by classname. This change uses a dedicated static id instead of the classname
to lookup the actual class.

@s1monw
Copy link
Contributor Author

s1monw commented Sep 17, 2015

since the exception mechanism is new in 2.0 I think this should go into 2.0GA since it prevents subtile bugs and is a minor change.


final int maxOrd = 140;
assert exceptions.size() == maxOrd + 1;
Constructor<? extends ElasticsearchException>[] mapping = new Constructor[maxOrd + 1];
Copy link
Contributor

Choose a reason for hiding this comment

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

can we call this idToClass?

@s1monw
Copy link
Contributor Author

s1monw commented Sep 17, 2015

@bleskes applied review comments

@bleskes
Copy link
Contributor

bleskes commented Sep 17, 2015

LGTM.

Classnames change quickly due to refactorings etc. If that happens in a minor release
we loose the ability to deserialize the exceptoin coming from another node sicne we today
look it up by classname. This change uses a dedicated static id instead of the classname
to lookup the actual class.
@s1monw s1monw merged commit af9166d into elastic:master Sep 17, 2015
@s1monw s1monw deleted the fix_exception_serialization branch September 17, 2015 11:45
@lcawl lcawl added :Core/Infra/Core Core issues without another label and removed :Exceptions labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants