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

[Backport 2.x] Deprecate class 'MasterService' and create alternative class 'ClusterManagerService' #4050

Merged
merged 1 commit into from
Jul 30, 2022

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

@opensearch-trigger-bot opensearch-trigger-bot bot commented Jul 30, 2022

Backport 740f75d from #4022

  • Deprecate class MasterService and create alternative class ClusterManagerService.
  • Add a unit test to validate the method ClusterService.getMasterService() can still return an object in type MasterService.
  • Rename all the existing references of MasterService to ClusterManagerService, and rename the local variable names.
  • Deprecate public methods ClusterServiceUtils.createMasterService(...) and create alternative methods createClusterManagerService(...)

…ManagerService' (#4022)

To support inclusive language, the `master` terminology is going to be replaced by `cluster manager` in the code base.

- Deprecate class `MasterService` and create alternative class `ClusterManagerService`.
- Add a unit test to validate the method `ClusterService.getMasterService()` can still return an object in type `MasterService`.
- Rename all the existing references of `MasterService` to `ClusterManagerService`, and rename the local variable names.
- Deprecate public methods `ClusterServiceUtils.createMasterService(...)` and create alternative methods `createClusterManagerService(...)`

Note:
The class `ClusterManagerService` is a subclass of `MasterService`, the inheritance relationship is opposite from most of the other classes with `Master` in the name (that covered by issue #1684).
The reason is:
There is a public method that has return value in type `MasterService`,
https://github.com/opensearch-project/OpenSearch/blob/388c80ad94529b1d9aad0a735c4740dce2932a32/server/src/main/java/org/opensearch/cluster/service/ClusterService.java#L221
And in the code for Performance Analyzer plugin, there is a local variable in type `MasterService`:
https://github.com/opensearch-project/performance-analyzer/blob/5ee4809ac1cda6517ed871aeb12c6635203e7f1d/src/main/java/org/opensearch/performanceanalyzer/collectors/MasterServiceEventMetrics.java#L219
If making the old class `MasterService` a subclass of the new class `ClusterManagerService`, the above usage will be broken.
Reversing the inheritance relationship, I'm able to keep the backwards compatibility of the method `getMasterService()` while deprecating the class `MasterService` and encourage using a new class `ClusterManagerService`.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit 740f75d)
@opensearch-trigger-bot opensearch-trigger-bot bot requested review from a team and reta as code owners July 30, 2022 03:37
@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2022

Codecov Report

Merging #4050 (0566b79) into 2.x (e423ec4) will increase coverage by 0.05%.
The diff coverage is 71.11%.

@@             Coverage Diff              @@
##                2.x    #4050      +/-   ##
============================================
+ Coverage     70.48%   70.53%   +0.05%     
- Complexity    56470    56554      +84     
============================================
  Files          4532     4533       +1     
  Lines        272152   272158       +6     
  Branches      40003    40003              
============================================
+ Hits         191831   191976     +145     
+ Misses        64188    64059     -129     
+ Partials      16133    16123      -10     
Impacted Files Coverage Δ
...ster/tasks/TransportPendingClusterTasksAction.java 41.66% <0.00%> (ø)
...g/opensearch/cluster/ClusterStateTaskListener.java 60.00% <ø> (ø)
.../org/opensearch/cluster/service/MasterService.java 83.19% <ø> (ø)
...rg/opensearch/common/settings/ClusterSettings.java 91.89% <ø> (ø)
.../opensearch/common/util/concurrent/BaseFuture.java 62.71% <0.00%> (ø)
...java/org/opensearch/discovery/DiscoveryModule.java 90.27% <ø> (ø)
...h/cluster/service/FakeThreadPoolMasterService.java 88.88% <ø> (ø)
...ch/test/disruption/BlockMasterServiceOnMaster.java 0.00% <0.00%> (ø)
...h/test/disruption/BusyMasterServiceDisruption.java 0.00% <0.00%> (ø)
.../java/org/opensearch/test/ClusterServiceUtils.java 50.00% <27.27%> (-0.62%) ⬇️
... and 498 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants