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

Replace "master" terminology in Java APIs #1684

Open
3 of 4 tasks
tlfeng opened this issue Dec 9, 2021 · 1 comment
Open
3 of 4 tasks

Replace "master" terminology in Java APIs #1684

tlfeng opened this issue Dec 9, 2021 · 1 comment
Labels
>breaking Identifies a breaking change. enhancement Enhancement or improvement to existing feature or request Meta Meta issue, not directly linked to a PR v3.0.0 Issues and PRs related to version 3.0.0

Comments

@tlfeng
Copy link
Collaborator

tlfeng commented Dec 9, 2021

Is your feature request related to a problem? Please describe.
Replace the "master" terminology in all Java APIs, including field, method, class, and package names.
The "Java APIs" refers to those packaged in Java libraries and are published to Maven (https://search.maven.org/search?q=g:org.opensearch https://mvnrepository.com/artifact/org.opensearch)

Impact:
All plugins, clients and tools that use OpenSearch Java APIs from OpenSearch Java libraries which contain non-inclusive terminologies have to make corresponding changes to call new APIs, if they want to upgrade the dependency to a future major version of OpenSearch.

A part of #472

Describe the solution you'd like
Replace the "master" terminology with "ClusterManager" in all Java APIs that are exposed in Java libraries.

Overall solution: #1684 (comment)
Sub-issue:

Describe alternatives you've considered
None.

Additional context
Locations of "master" in API of Java library:
These libraries have been published to Maven. (Links: https://mvnrepository.com/artifact/org.opensearch https://search.maven.org/search?q=g:org.opensearch)

server
1 package org.opensearch.action.support.master
https://opensearch.org/javadocs/1.1.0/OpenSearch/server/build/docs/javadoc/org/opensearch/action/support/master/package-summary.html
(totally 1 package, the package contains 15 Java classes)
2 class org.opensearch.cluster.MasterNodeChangePredicate
https://opensearch.org/javadocs/2.0.0/OpenSearch/server/build/docs/javadoc/org/opensearch/cluster/MasterNodeChangePredicate.html
(totally 14 classes)
3 variable org.opensearch.action.support.master.MasterNodeRequest.DEFAULT_MASTER_NODE_TIMEOUT
https://opensearch.org/javadocs/1.1.0/OpenSearch/server/build/docs/javadoc/org/opensearch/action/support/master/MasterNodeRequest.html#DEFAULT_MASTER_NODE_TIMEOUT
(totally 15 variables)
4 method org.opensearch.cluster.node.DiscoveryNodes.Builder.masterNodeId(String)
https://opensearch.org/javadocs/2.0.0/OpenSearch/server/build/docs/javadoc/org/opensearch/cluster/node/DiscoveryNodes.Builder.html#masterNodeId(java.lang.String)
... (In version 1.1.0, totally 177 methods) (In version 2.0.0, totally 142 methods)

Java Low Level REST Client
1 variable org.opensearch.client.NodeSelector.SKIP_DEDICATED_MASTERS
https://opensearch.org/javadocs/1.1.0/OpenSearch/client/rest/build/docs/javadoc/org/opensearch/client/NodeSelector.html#SKIP_DEDICATED_MASTERS
(totally 1 variable)
2 method org.opensearch.client.Node.Roles.isMasterEligible()
https://opensearch.org/javadocs/1.1.0/OpenSearch/client/rest/build/docs/javadoc/org/opensearch/client/Node.Roles.html#isMasterEligible()
(totally 1 method)

Java High Level REST Client
1 variable org.opensearch.client.TimedRequest.DEFAULT_MASTER_NODE_TIMEOUT
https://opensearch.org/javadocs/1.1.0/OpenSearch/client/rest-high-level/build/docs/javadoc/org/opensearch/client/TimedRequest.html#DEFAULT_MASTER_NODE_TIMEOUT
(totally 1 variable)
2 method org.opensearch.client.TimedRequest.masterNodeTimeout()
https://opensearch.org/javadocs/2.0.0/OpenSearch/client/rest-high-level/build/docs/javadoc/org/opensearch/client/TimedRequest.html#masterNodeTimeout()
(totally 11 methods)

Test Framework
1 class org.opensearch.test.disruption.BlockMasterServiceOnMaster
https://opensearch.org/javadocs/2.0.0/OpenSearch/test/framework/build/docs/javadoc/org/opensearch/test/disruption/BlockMasterServiceOnMaster.html
(totally 3 classes)
2 variable org.opensearch.test.InternalTestCluster.DEFAULT_HIGH_NUM_MASTER_NODES
https://opensearch.org/javadocs/2.0.0/OpenSearch/test/framework/build/docs/javadoc/org/opensearch/test/InternalTestCluster.html#DEFAULT_HIGH_NUM_MASTER_NODES
(totally 4 variables)
3 method org.opensearch.client.Client.masterClient()
https://opensearch.org/javadocs/2.0.0/OpenSearch/test/framework/build/docs/javadoc/org/opensearch/test/InternalTestCluster.html#masterClient()
(totally 41 methods)

@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request untriaged >breaking Identifies a breaking change. and removed untriaged labels Dec 9, 2021
@tlfeng tlfeng added the v3.0.0 Issues and PRs related to version 3.0.0 label May 13, 2022
@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 3, 2022

The plan to resolve the issue:

Result:

  • On 2.x branch:
  1. There are deprecation notice to all public and protected class/method/variable names in the code, which contain "master" terminology. It includes
    a. @Deprecated annotation
    b. @deprecated tag in Javadoc, @deprecated As of 2.1, because supporting inclusive language, replaced by {@link <new_class>#<new_method>}
  2. There are alternative package/class/method/variable that "master" is replaced by "cluster manager" in the name.
  • On main branch:
  1. The "master" word is replaced by "cluster manager" in all public and protected class/method/variable names.

Implementation:
On main branch, and for each public and protected class/method/variable which contains "master" terminology,

  1. Copy the codes of definition.
  2. Replace "master" word with "cluster manager" in its name. This is done by the Rename refactoring feature of IntelliJ IDEA, so that both the definition and reference can be renamed.
  3. Paste the original codes of the definition aside.
  4. Add @Deprecated annotation and @deprecated tag in Javadoc
  5. Remove all the existing implementation in the old class or method, to keep the old usage for the deprecated classes and methods while only maintain the new renamed classes and methods.
    For class, make the old class extends the new renamed class.
    For package, apply the class change to all classes in the package.
    For method, call the new renamed method in the old method.
  6. If the renamed class has unit test, copy the unit test to test the deprecated class.
  7. Backport all the changes to 2.x branch.
  8. Remove all the deprecated class/method/variable.

Note:

  1. No new test will be added.
  2. The deprecation and renaming should be done in the order of "package -> class -> method and variable", so that the new package or class will have no deprecated class or method, which makes the code clean.
  3. The class/method/variable that used for maintaining the compatibility of "master" usage in REST APIs and settings will not be deprecated or renamed.
  4. The deprecation notice in the code will be added from the next minor version (2.1), and the deprecated items will be removed in next major version (3.0).

Appendix
The regex to filter the lines of code with public Java API contains "master" terminology:
class: (public|protected)(.)+(class|interface)\s(\w)*Master
method: (public|protected)(.)+[Mm]aster(\w)*\(
variable: (public|protected)(.)+(MASTER|master)(.)*=

Renaming rule:

  • variable name:
    master -> clusterManager
    MASTER -> CLUSTER_MANAGER
  • method name:
    master -> clusterManager
    Master -> ClusterManager
  • class/package name:
    Master-> ClusterManager

@tlfeng tlfeng added the Meta Meta issue, not directly linked to a PR label Jun 8, 2022
tlfeng pushed a commit that referenced this issue Jul 30, 2022
…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 <ftianli@amazon.com>
opensearch-trigger-bot bot pushed a commit that referenced this issue Jul 30, 2022
…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 <ftianli@amazon.com>
(cherry picked from commit 740f75d)
tlfeng pushed a commit that referenced this issue Jul 30, 2022
…ManagerService' (#4022) (#4050)

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 <ftianli@amazon.com>
(cherry picked from commit 740f75d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking Identifies a breaking change. enhancement Enhancement or improvement to existing feature or request Meta Meta issue, not directly linked to a PR v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

No branches or pull requests

1 participant