Read min cluster version directly from DiscoveryNodes #581
Conversation
This file contains 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
Signed-off-by: Martin Gaievski <[email protected]>
martin-gaievski
added
Enhancements
Codecov Report
@@ Coverage Diff @@
## feature/efficient-filtering #581 +/- ##
=================================================================
- Coverage 84.38% 84.09% -0.29%
+ Complexity 1055 1048 -7
=================================================================
Files 149 149
Lines 4316 4308 -8
Branches 384 382 -2
=================================================================
- Hits 3642 3623 -19
- Misses 499 508 +9
- Partials 175 177 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
heemin32
reviewed
Oct 14, 2022
| @@ -49,20 +46,11 @@ public void initialize(final ClusterService clusterService) { | |||
| */ | |||
| public Version getClusterMinVersion() { | |||
| Version minVersion = Version.CURRENT; | |||
There was a problem hiding this comment.
Suggested change
| Version minVersion = Version.CURRENT; | |
| public Version getClusterMinVersion() { | |
| try { | |
| return this.clusterService.state().getNodes().getMinNodeVersion(); | |
| } catch (Exception exception) { | |
| log.error("Failed to get minimum node version of the cluster. Returning current node version instead.", exception); | |
| return Version.CURRENT; | |
| } | |
| } |
There was a problem hiding this comment.
Trivial. Better not to change variable value once it is defined to reduce a chance of an error.
| try { | ||
| clusterDiscoveryNodes = this.clusterService.state().getNodes().getNodes(); | ||
| minVersion = this.clusterService.state().getNodes().getMinNodeVersion(); | ||
| } catch (Exception exception) { | ||
| log.error("Cannot get cluster nodes", exception); |
There was a problem hiding this comment.
Error message needs to be updated.
Signed-off-by: Martin Gaievski <[email protected]>
heemin32
approved these changes
Oct 14, 2022
VijayanB
approved these changes
Oct 14, 2022
martin-gaievski
merged commit 9b32e17
into
opensearch-project:feature/efficient-filtering
martin-gaievski
added a commit
to martin-gaievski/k-NN
that referenced
this pull request
Oct 20, 2022
…ject#581) * Simplify min cluster version lookup Signed-off-by: Martin Gaievski <[email protected]>
1 task
heemin32
added
v2.4.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: Martin Gaievski [email protected]
Description
Found simpler way of reading min deployed version for cluster, we can remove direct iteration over cluster nodes - https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/cluster/node/DiscoveryNodes.java#L399. Changing that for context of filtering feature.
Issues Resolved
#376
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.