add new role for Offline Nodes #13613
Conversation
Signed-off-by: Varun Bansal <bansvaru@amazon.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13613 +/- ##
============================================
+ Coverage 71.42% 71.47% +0.05%
- Complexity 59978 61054 +1076
============================================
Files 4985 5052 +67
Lines 282275 287155 +4880
Branches 40946 41608 +662
============================================
+ Hits 201603 205258 +3655
- Misses 63999 64946 +947
- Partials 16673 16951 +278 ☔ View full report in Codecov by Sentry. |
| public static boolean isOfflineNode(Settings settings) { | ||
| return hasRole(settings, DiscoveryNodeRole.OFFLINE_ROLE); | ||
| } | ||
|
|
There was a problem hiding this comment.
DiscoveryNode has instance methods for these too. Do we need one for the offline role?
| @@ -391,6 +392,20 @@ public NodeEnvironment(Settings settings, Environment environment, IndexStoreLis | |||
| ensureNoFileCacheData(fileCacheNodePath); | |||
| } | |||
|
|
|||
| if (DiscoveryNode.isOfflineNode(settings) && DiscoveryNode.getRolesFromSettings(settings).size() > 1) { | |||
There was a problem hiding this comment.
Please could you help add tests for this?
There was a problem hiding this comment.
tests are pending. will be working on adding those
|
I am guessing the intent of an offline node is to execute offline tasks. The naming is confusing. A node that's offline is ... offline. I think a better name could be a "worker" or "task" node that is capable of executing tasks, and doesn't serve other purpose than that. The node itself is not offline. |
Thanks for the feedback @dblock . I agree Offline does create some confusion and did spend some time initially to think of a better name but didn't have much luck. But let me give it another shot and come up with a better keyword than "Offline". If you are interested in getting more context around Offline Nodes, here is a detailed design doc #13554 and a PR introducing the base library #13574 |
|
This PR is stalled because it has been open for 30 days with no activity. |
Description
[Describe what this change achieves]
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
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.