Add BWC layer for Exceptions #21694
Merged
Add BWC layer for Exceptions #21694
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today it's not possible to add exceptions to the serialization layer without breaking BWC. This commit adds the ability to specify the Version an exception was added that allows to fall back not NotSerializableExceptionWrapper if the expection is not present in the streams version. Relates to elastic#21656
s1monw
added
:Core/Infra/Core
Contributor
Author
|
@ywelsch can you take a look |
clintongormley
added
>enhancement
and removed
:Core/Infra/Core
ywelsch
approved these changes
Nov 21, 2016
Contributor
ywelsch
left a comment
ywelsch
left a comment
There was a problem hiding this comment.
LGTM, I think the change in Store should be reverted though.
| } catch (FileNotFoundException | NoSuchFileException ex) { | ||
| logger.info("Failed to open / find files while reading metadata snapshot"); | ||
| } catch (ShardLockObtainFailedException ex) { | ||
| logger.info((Supplier<?>) () -> new ParameterizedMessage("{}: failed to obtain shard lock", shardId), ex); |
Contributor
There was a problem hiding this comment.
This is a tricky change to make and I want to understand the full consequences first. If I follow the flow of the code correctly, this code is called when a master wants to allocate a replica and checks if there are nodes that have data for the replica. Making this change here means that the fetching for this node will be considered as failed and will trigger a reroute in AsyncShardFetch. The replica might however still be allocated in ReplicaShardAllocator to another node or later allocated on any node in BalancedShardsAllocator. I'm not sure if that's the change we want. Can you leave this code as is for now?
Contributor
Author
There was a problem hiding this comment.
ok sure - I copied this from your commit 👯♂️
Contributor
Author
|
@ywelsch I rolled back the changes to Store.java |
s1monw
added a commit
that referenced
this pull request
Nov 21, 2016
Today it's not possible to add exceptions to the serialization layer without breaking BWC. This commit adds the ability to specify the Version an exception was added that allows to fall back not NotSerializableExceptionWrapper if the exception is not present in the streams version. Relates to #21656
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Nov 22, 2016
* master: (42 commits) Add support for merging custom meta data in tribe node (elastic#21552) [DOCS] Show EC2's auto attribute (elastic#21474) Add information about the removal of store throttling to the migration guide. Add a recommendation against large documents to the docs. (elastic#21652) Add indices options tests to search api REST tests (elastic#21701) Fixing indentation in geospatial querying example. (elastic#21682) Fix typo in filters aggregation docs (elastic#21690) Add BWC layer for Exceptions (elastic#21694) Add checkstyle rule to forbid empty javadoc comments (elastic#20881) Docs: Added offline install link for discovery-file plugin remove pointless catch exception in TransportSearchAction (elastic#21689) Rename ClusterState#lookupPrototypeSafe to `lookupPrototype` and remove previous "unsafe" unused variant (elastic#21686) Use a buffer to do character to byte conversion in StreamOutput#writeString (elastic#21680) Fix integer overflows when dealing with templates. (elastic#21628) Fix highlighting on a stored keyword field (elastic#21645) Set execute permissions for native plugin programs (elastic#21657) adjust visibility of DiscoveryNodes.Delta constructor Remove unused DiscoveryNodes.Delta constructor Remove unused DiscoveryNode#removeDeadMembers public method Remove minNodeVersion and corresponding public `getSmallestVersion` getter method from DiscoveryNodes ...
ywelsch
pushed a commit
that referenced
this pull request
Nov 22, 2016
Today it's not possible to add exceptions to the serialization layer without breaking BWC. This commit adds the ability to specify the Version an exception was added that allows to fall back not NotSerializableExceptionWrapper if the exception is not present in the streams version. Relates to #21656
Contributor
|
5.0.2 backport: 83f6e84 |
Contributor
|
adaptation to v6.0.0 to support 5.0.2 backport: c521219 |
Contributor
Author
|
thanks @ywelsch |
Contributor
|
adaptation to v5.1.0 to support 5.0.2 backport: 05529d9 |
lcawl
added
:Core/Infra/Core
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today it's not possible to add exceptions to the serialization layer
without breaking BWC. This commit adds the ability to specify the Version
an exception was added that allows to fall back not NotSerializableExceptionWrapper
if the exception is not present in the streams version.
Relates to #21656