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

Rename public classes with 'Master' to 'ClusterManager' #3619

Merged
merged 9 commits into from
Jul 12, 2022

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Jun 17, 2022

Description

Replace master terminology by cluster manager in the public Java APIs to support inclusive language.
The PR deal with the public class name in the repository (except for those covered in issue #3542).

  • Replace Master to ClusterManager for all the classes, including all the references to the classes.

The above changes are done by the Rename refactoring feature of IntelliJ IDEA, so that both the definition and reference can be renamed.

The next PR will be like de21446, adding back the classes in old name for backwards compatibility.

List of classes that renamed in this PR:
sever directory:
interface LocalNodeMasterListener -> LocalNodeClusterManagerListener
final class MasterNodeChangePredicate -> ClusterManagerNodeChangePredicate
NotMasterException -> NotClusterManagerException
NoMasterBlockService -> NoClusterManagerBlockService
UnsafeBootstrapMasterCommand - UnsafeBootstrapClusterManagerCommand
MasterService -> ClusterManagerService
MasterNotDiscoveredException -> ClusterManagerNotDiscoveredException
RestMasterAction -> RestClusterManagerAction
test/framework directory:
FakeThreadPoolMasterService -> FakeThreadPoolClusterManagerService
BlockMasterServiceOnMaster -> BlockClusterManagerServiceOnMaster
BusyMasterServiceDisruption -> BusyClusterManagerServiceDisruption

Issues Resolved

The first step to resolve issue #3543

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request v2.1.0 Issues and PRs related to version 2.1.0 v3.0.0 Issues and PRs related to version 3.0.0 backport 2.x Backport to 2.x branch labels Jun 17, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure f4d28f1
Log 6091

Reports 6091

@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 17, 2022

Log 6091 shows the test failure in issue #3579

Tianli Feng added 5 commits June 21, 2022 22:23
Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/internalClusterTest/java/org/opensearch/action/admin/indices/exists/IndicesExistsIT.java
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure a0b3f74
Log 6206

Reports 6206

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 5f55097
Log 6207

Reports 6207

@tlfeng tlfeng marked this pull request as ready for review June 22, 2022 06:41
@tlfeng tlfeng requested review from a team and reta as code owners June 22, 2022 06:41
@tlfeng tlfeng marked this pull request as draft June 22, 2022 06:42
@tlfeng tlfeng marked this pull request as ready for review June 22, 2022 16:22
@@ -277,7 +277,7 @@ public void removeTimeoutListener(TimeoutClusterStateListener listener) {
/**
* Add a listener for on/off local node cluster-manager events
*/
public void addLocalNodeMasterListener(LocalNodeMasterListener listener) {
public void addLocalNodeMasterListener(LocalNodeClusterManagerListener listener) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think addLocalNodeMasterListener method should either be renamed as part of this PR to addLocalNodeClusterManagerListener or should be documented/added in issue #3543 in the second part of the issue.

Copy link
Collaborator Author

@tlfeng tlfeng Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rename the method addLocalNodeMasterListener in issue #3544, which is being changed in PR #3647.
In issue #3543, only the methods within the above list of classes will be renamed, and of course, in a following PR as well.
There will be 3 PRs to address #3543:

  1. Rename "Master" to "ClusterManager" in the classes names (this one)
  2. Add the classes with the old "Master" name back to keep the compatibility
  3. Rename and deprecate the public methods and variables in the above classes.

clusterApplierService.addLocalNodeMasterListener(listener);
}

public MasterService getMasterService() {
return masterService;
public ClusterManagerService getMasterService() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getMasterService() should be renamed in this PR to getClusterManagerService() or this change should be documented in the parent issue #3543 under part 2

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rename it when resolving issue #3544, which is being changed in PR #3647.

@@ -77,7 +77,7 @@ public static MasterService createMasterService(ThreadPool threadPool, ClusterSt
return clusterManagerService;
}

public static MasterService createMasterService(ThreadPool threadPool, DiscoveryNode localNode) {
public static ClusterManagerService createMasterService(ThreadPool threadPool, DiscoveryNode localNode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming of createMasterService to createClusterManagerService must be documented in parent issue #3543

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I will rename it when resolving issue #3544

@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 22, 2022

Hi @Poojita-Raj, thank you so much for reviewing my PR!

Copy link
Contributor

@Poojita-Raj Poojita-Raj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for linking me to the issues that will be covering these methods' renaming. LGTM!

@tlfeng
Copy link
Collaborator Author

tlfeng commented Jun 23, 2022

@Poojita-Raj Thanks a again for your approval! 👍👍
Please hold on backporting this change to 2.x branch, since I need to revise the plan for the renamed classes to make sure the compatibility is kept.

@tlfeng tlfeng added v2.2.0 and removed v2.1.0 Issues and PRs related to version 2.1.0 labels Jun 24, 2022
Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a merge conflict and a couple PR comments that still need to be addressed but otherwise LGTM.

Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/internalClusterTest/java/org/opensearch/discovery/ClusterManagerDisruptionIT.java
#	server/src/main/java/org/opensearch/cluster/coordination/NoClusterManagerBlockService.java
@tlfeng tlfeng force-pushed the rename-master-class branch from 7267e52 to fdb869d Compare July 11, 2022 18:39
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@tlfeng tlfeng merged commit a7e113a into opensearch-project:main Jul 12, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 12, 2022
Replace master terminology by cluster manager in the public Java APIs to support inclusive language.
The PR deal with the public class name in the repository (except for those covered in issue #3542).

* Replace Master to ClusterManager for all the classes, including all the references to the classes.

The next PR will be like de21446, adding back the classes in old name for backwards compatibility.

List of classes that renamed in this PR:
sever directory:
interface LocalNodeMasterListener -> LocalNodeClusterManagerListener
final class MasterNodeChangePredicate -> ClusterManagerNodeChangePredicate
NotMasterException -> NotClusterManagerException
NoMasterBlockService -> NoClusterManagerBlockService
UnsafeBootstrapMasterCommand - UnsafeBootstrapClusterManagerCommand
MasterService -> UnsafeBootstrapClusterManagerCommand
MasterNotDiscoveredException -> ClusterManagerNotDiscoveredException
RestMasterAction -> RestClusterManagerAction
test/framework directory:
FakeThreadPoolMasterService -> FakeThreadPoolClusterManagerService
BlockMasterServiceOnMaster -> BlockClusterManagerServiceOnMaster
BusyMasterServiceDisruption -> BusyClusterManagerServiceDisruption

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit a7e113a)
@tlfeng tlfeng deleted the rename-master-class branch July 12, 2022 21:52
tlfeng pushed a commit that referenced this pull request Jul 14, 2022
#3870)

* Rename public classes with 'Master' to 'ClusterManager' (#3619)

Replace master terminology by cluster manager in the public Java APIs to support inclusive language.
The PR deal with the public class name in the repository (except for those covered in issue #3542).

* Replace `Master` to `ClusterManager` in most of the class names (except `MasterService` class), including all the references to the classes.

The next PR will be #3871, adding back the classes in old name for backwards compatibility.

List of classes that renamed in this PR:
`sever` directory:
interface LocalNodeMasterListener -> LocalNodeClusterManagerListener
final class MasterNodeChangePredicate -> ClusterManagerNodeChangePredicate
NotMasterException -> NotClusterManagerException
NoMasterBlockService -> NoClusterManagerBlockService
UnsafeBootstrapMasterCommand - UnsafeBootstrapClusterManagerCommand
MasterNotDiscoveredException -> ClusterManagerNotDiscoveredException
RestMasterAction -> RestClusterManagerAction

List of classes that not renamed:
`sever` directory:
MasterService
`test/framework` directory:
FakeThreadPoolMasterService
BlockMasterServiceOnMaster
BusyMasterServiceDisruption

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit a7e113a)
tlfeng pushed a commit that referenced this pull request Jul 14, 2022
- Add back the usage of old names for some classes for backwards compatibility. Those public classes were renamed from "master" to "cluster manager" In PR #3619 / commit a7e113a.
 - Add a unit test to validate the backwards compatibility of interface `LocalNodeMasterListener` remains.
 - Revert the renaming of class `MasterService`, `FakeThreadPoolMasterService`, `BlockMasterServiceOnMaster` and `BusyMasterServiceDisruption`, as well as instance variable names of class `MasterService`. 
 Because I couldn't solve the compatibility problem of a public method `public ClusterManagerService getMasterService()` (#3871 (comment))

**List of classes that renamed in previous PR:**
In this PR, the usages of the old names should all be restored.
`sever` directory:
interface LocalNodeMasterListener -> LocalNodeClusterManagerListener
final class MasterNodeChangePredicate -> ClusterManagerNodeChangePredicate
NotMasterException -> NotClusterManagerException
NoMasterBlockService -> NoClusterManagerBlockService
UnsafeBootstrapMasterCommand - UnsafeBootstrapClusterManagerCommand
MasterNotDiscoveredException -> ClusterManagerNotDiscoveredException
RestMasterAction -> RestClusterManagerAction

**List of classes that not renamed:**
`sever` directory:
MasterService
`test/framework` directory:
FakeThreadPoolMasterService
BlockMasterServiceOnMaster
BusyMasterServiceDisruption

Signed-off-by: Tianli Feng <[email protected]>
tlfeng pushed a commit to tlfeng/OpenSearch that referenced this pull request Jul 15, 2022
…ect#3871)

- Add back the usage of old names for some classes for backwards compatibility. Those public classes were renamed from "master" to "cluster manager" In PR opensearch-project#3619 / commit opensearch-project@a7e113a.
 - Add a unit test to validate the backwards compatibility of interface `LocalNodeMasterListener` remains.
 - Revert the renaming of class `MasterService`, `FakeThreadPoolMasterService`, `BlockMasterServiceOnMaster` and `BusyMasterServiceDisruption`, as well as instance variable names of class `MasterService`.
 Because I couldn't solve the compatibility problem of a public method `public ClusterManagerService getMasterService()` (opensearch-project#3871 (comment))

**List of classes that renamed in previous PR:**
In this PR, the usages of the old names should all be restored.
`sever` directory:
interface LocalNodeMasterListener -> LocalNodeClusterManagerListener
final class MasterNodeChangePredicate -> ClusterManagerNodeChangePredicate
NotMasterException -> NotClusterManagerException
NoMasterBlockService -> NoClusterManagerBlockService
UnsafeBootstrapMasterCommand - UnsafeBootstrapClusterManagerCommand
MasterNotDiscoveredException -> ClusterManagerNotDiscoveredException
RestMasterAction -> RestClusterManagerAction

**List of classes that not renamed:**
`sever` directory:
MasterService
`test/framework` directory:
FakeThreadPoolMasterService
BlockMasterServiceOnMaster
BusyMasterServiceDisruption

Signed-off-by: Tianli Feng <[email protected]>
tlfeng pushed a commit that referenced this pull request Jul 15, 2022
…3914)

* Deprecate public class names with master terminology (#3871)

- Add back the usage of old names for some classes for backwards compatibility. Those public classes were renamed from "master" to "cluster manager" In PR #3619 / commit a7e113a.
 - Add a unit test to validate the backwards compatibility of interface `LocalNodeMasterListener` remains.
 - Revert the renaming of class `MasterService`, `FakeThreadPoolMasterService`, `BlockMasterServiceOnMaster` and `BusyMasterServiceDisruption`, as well as instance variable names of class `MasterService`.
 Because I couldn't solve the compatibility problem of a public method `public ClusterManagerService getMasterService()` (#3871 (comment))

**List of classes that renamed in previous PR:**
In this PR, the usages of the old names should all be restored.
`sever` directory:
interface LocalNodeMasterListener -> LocalNodeClusterManagerListener
final class MasterNodeChangePredicate -> ClusterManagerNodeChangePredicate
NotMasterException -> NotClusterManagerException
NoMasterBlockService -> NoClusterManagerBlockService
UnsafeBootstrapMasterCommand - UnsafeBootstrapClusterManagerCommand
MasterNotDiscoveredException -> ClusterManagerNotDiscoveredException
RestMasterAction -> RestClusterManagerAction

**List of classes that not renamed:**
`sever` directory:
MasterService
`test/framework` directory:
FakeThreadPoolMasterService
BlockMasterServiceOnMaster
BusyMasterServiceDisruption

Signed-off-by: Tianli Feng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch enhancement Enhancement or improvement to existing feature or request v2.2.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants