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

Deprecate package 'org.opensearch.action.support.master' #3542

Closed
tlfeng opened this issue Jun 8, 2022 · 3 comments
Closed

Deprecate package 'org.opensearch.action.support.master' #3542

tlfeng opened this issue Jun 8, 2022 · 3 comments
Assignees
Labels
deprecate enhancement Enhancement or improvement to existing feature or request v2.2.0

Comments

@tlfeng
Copy link
Collaborator

tlfeng commented Jun 8, 2022

Is your feature request related to a problem? Please describe.
A part of issue #1684.
To support inclusive language, the master terminology is going to be replaced by cluster manager in the code base.

The goal for this issue is:

  1. Deprecate the package org.opensearch.action.support.master, which is the only package contains "master" in the name, and it will be removed in the next major version (3.0)
  2. Have alternative package org.opensearch.action.support.clustermanager as the replacement.
  3. Rename all public/protected classes, methods and variable contain "master" in the old package, and have alternative new class, method or variable name in the new package. Each class in the new package preserves the git history of the deprecated class as the successor.

Describe the solution you'd like
For overall solution to replace "master" in public Java APIs: #1684 (comment)

Key points:

  1. At the time of deprecating the package, all public/protected classes, methods or variables that contain "master" in the package will be deprecated as well, to avoid spending time modifying the same file again.
  2. Any public/protected Class with "master" in the name will extend the renamed class to maintain only one class while supporting the old class.
  3. Any public/protected method or variable with "master" in the name will directly call the renamed method or variable, for the same reason as above.
  4. Renaming and deprecating class/method/variable must be in 2 separate Pull Requests, in order to be in separate git commits after PR merged.
    Because there will be 2 classes as the result, one with new the name and one with the old name, and the class with new name should be the successor, only in this way can preserve the git file history in the renamed class.

In detail:

  1. Copy the folder of the package and keep it in another place.
  2. Replace master word with clustermanager in the package name. This is done by the Rename refactoring feature of IntelliJ IDEA, so that both the definition and reference can be renamed.
  3. Replace Master with ClusterManager in the class name, using the same way as above.
  4. replace master with clusterManager in the method name, using the same way as above.
  5. Create a Pull Request for the above changes, so that the file renaming can be tracked in the git history.
  6. Wait until the PR merged.
  7. Paste the original master folder back.
  8. Add @Deprecated annotation and @deprecated Javadoc tag in package-info.java to mark the whole package as deprecated.
  9. Remove all the existing implementation in the old classes, except any public/protected method or variable that contains "master" name. Make the old class extends the new renamed class.
    For class, adding all the constructors of the super class.
    For the method or variable contains inclusive name, change its implementation to call the corresponding ones in super class.
  10. Add @Deprecated annotation and @deprecated Javadoc tag for the class and method that contains "master" in the name.
  11. If the renamed class has unit test, copy the unit test to test the deprecated class.
  12. Backport all the changes to 2.x branch.

Example of the 2 commits:
In branch: https://github.com/tlfeng/OpenSearch/commits/rename-master-package-preview
First step: rename - tlfeng@cdc3de4
Second step: add old usage back and deprecate: tlfeng@e484e0f

Describe alternatives you've considered
None.

Additional context
The package org.opensearch.action.support.master has got 15 Java classes inside.
6 classes of those having "master" in the names.
8 methods and 2 variables in these classes having "master" in the names.

1 Class to be renamed:
MasterNodeOperationRequestBuilder
MasterNodeReadOperationRequestBuilder
MasterNodeReadRequest
MasterNodeRequest
TransportMasterNodeAction
TransportMasterNodeReadAction

2 Method and variable to be renamed:
In class MasterNodeRequest -
TimeValue DEFAULT_MASTER_NODE_TIMEOUT
TimeValue masterNodeTimeout
Request masterNodeTimeout(TimeValue timeout)
Request masterNodeTimeout(String timeout)
TimeValue masterNodeTimeout()

In class TransportMasterNodeAction -
void masterOperation(Request request, ClusterState state, ActionListener listener)
void masterOperation(Task task, Request request, ClusterState state, ActionListener listener)
String getMasterActionName(DiscoveryNode node)

In class TransportClusterInfoAction -
void masterOperation(final Request request, final ClusterState state, final ActionListener listener)
void doMasterOperation(Request request, String[] concreteIndices, ClusterState state, ActionListener listener)

@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request untriaged deprecate v2.1.0 Issues and PRs related to version 2.1.0 and removed untriaged labels Jun 8, 2022
@tlfeng tlfeng self-assigned this Jun 13, 2022
rupanshisharma pushed a commit to rupanshisharma/index-management that referenced this issue Jun 23, 2022
@tlfeng tlfeng added v2.2.0 and removed v2.1.0 Issues and PRs related to version 2.1.0 labels Jun 23, 2022
rupanshisharma pushed a commit to rupanshisharma/index-management that referenced this issue Jun 23, 2022
rupanshisharma added a commit to opensearch-project/index-management that referenced this issue Jun 24, 2022
* version upgrade to 2.1.0

Signed-off-by: Rupanshi Sharma <[email protected]>

* Renames master/Master to clusterManager/ClusterManager as mentioned here: opensearch-project/OpenSearch#3542

Signed-off-by: Rupanshi Sharma <[email protected]>

* Change XContentType.fromMediaTypeOrFormat to XContentType.fromMediaType in a test class to fix the unresolved method name error

Signed-off-by: Rupanshi Sharma <[email protected]>

* Upgrade gradle jar

Signed-off-by: Rupanshi Sharma <[email protected]>

* Revert "Renames master/Master to clusterManager/ClusterManager as mentioned here: opensearch-project/OpenSearch#3542"

This reverts commit 01dd024.

Signed-off-by: Rupanshi Sharma <[email protected]>

Co-authored-by: Rupanshi Sharma <[email protected]>
tlfeng pushed a commit that referenced this issue 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]>
opensearch-trigger-bot bot pushed a commit that referenced this issue 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 pushed a commit that referenced this issue 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
Copy link
Collaborator Author

tlfeng commented Jul 20, 2022

Progress update on 07/20/2022:
To achieve the goal to deprecate the package org.opensearch.action.support.master, and use org.opensearch.action.support.clustermanager instead:
Completed:
The following classes that have been deprecated and alternative classes have been created:

MasterNodeOperationRequestBuilder
MasterNodeReadOperationRequestBuilder
MasterNodeReadRequest
MasterNodeRequest
TransportMasterNodeAction
TransportMasterNodeReadAction
ClusterInfoRequest (moved to new package without renaming)
ClusterInfoRequestBuilder (moved to new package without renaming)
TrasnportClusterInfoAction (moved to new package without renaming)

The following methods have been deprecated and alternative have been created:
In class MasterNodeRequest -

TimeValue DEFAULT_MASTER_NODE_TIMEOUT
TimeValue masterNodeTimeout
Request masterNodeTimeout(TimeValue timeout)
Request masterNodeTimeout(String timeout)
TimeValue masterNodeTimeout()

In class TransportMasterNodeAction -

String getMasterActionName(DiscoveryNode node)

Remaining:
The following classes are pending to be deprecated and renamed:

AcknowledgedRequest
AcknowledgedRequestBuilder
AcknowledgedResponse
ShardsAcknowledgedResponse

The following methods are pending to be deprecated and renamed:
In class TransportMasterNodeAction -

abstract void masterOperation(Request request, ClusterState state, ActionListener listener)
void masterOperation(Task task, Request request, ClusterState state, ActionListener<Response> listener)

In class TransportClusterInfoAction -

abstract void doMasterOperation(Request request, String[] concreteIndices, ClusterState state, ActionListener listener)
void masterOperation(final Request request, final ClusterState state, final ActionListener<Response> listener)

1 Reason for class AcknowledgedResponse is not deprecated:
There are many public methods in package org.opensearch.client have got return type of AcknowledgedResponse, such as:

public AcknowledgedResponse deleteComponentTemplate(DeleteComponentTemplateRequest req, RequestOptions options) throws IOException {

I haven't got a proper solution to keep the backwards compatibility for those methods while creating and encouraging to use new alternative class AcknowledgedResponse in org.opensearch.action.support.clustermanager package.
Related issue #3663 .

2 Reason for abstract method masterOperation() is not deprecated:
Related issue #3683. There has been a solution to deprecate the abstract method properly #3683 (comment), so there should be no blocker.

@tlfeng
Copy link
Collaborator Author

tlfeng commented Jul 30, 2022

Progress update on 07/29/2022:
Completed:
The following methods are deprecated and renamed by PR #4032 (backport to 2.x branch by #4048)

In class TransportMasterNodeAction -
abstract void masterOperation(Request request, ClusterState state, ActionListener listener)
void masterOperation(Task task, Request request, ClusterState state, ActionListener<Response> listener)

In class TransportClusterInfoAction -
abstract void doMasterOperation(Request request, String[] concreteIndices, ClusterState state, ActionListener listener)
void masterOperation(final Request request, final ClusterState state, final ActionListener<Response> listener)

Remaining:
The following classes still exist in package org.opensearch.action.support.master

AcknowledgedRequest
AcknowledgedRequestBuilder
AcknowledgedResponse
ShardsAcknowledgedResponse

But they will not be deprecated and provided an alternative usage, but be renamed in a future major version directly as a breaking change. There is the reason #3663 (comment)

@kartg kartg added v3.0.0 Issues and PRs related to version 3.0.0 and removed v2.2.0 labels Aug 3, 2022
@tlfeng tlfeng added v2.2.0 and removed v3.0.0 Issues and PRs related to version 3.0.0 labels Aug 4, 2022
@tlfeng
Copy link
Collaborator Author

tlfeng commented Aug 4, 2022

As mentioned above, the 4 classes (AcknowledgedRequest, AcknowledgedRequestBuilder, AcknowledgedResponse, ShardsAcknowledgedResponse) in package org.opensearch.action.support.master will not be deprecated in version 2.x, and will be moved to package org.opensearch.action.support.clustermanager directly in a future major version as a breaking change.
Other than that, there is no remaining for this issue.

@tlfeng tlfeng closed this as completed Aug 4, 2022
wuychn pushed a commit to ochprince/index-management that referenced this issue Mar 16, 2023
* version upgrade to 2.1.0

Signed-off-by: Rupanshi Sharma <[email protected]>

* Renames master/Master to clusterManager/ClusterManager as mentioned here: opensearch-project/OpenSearch#3542

Signed-off-by: Rupanshi Sharma <[email protected]>

* Change XContentType.fromMediaTypeOrFormat to XContentType.fromMediaType in a test class to fix the unresolved method name error

Signed-off-by: Rupanshi Sharma <[email protected]>

* Upgrade gradle jar

Signed-off-by: Rupanshi Sharma <[email protected]>

* Revert "Renames master/Master to clusterManager/ClusterManager as mentioned here: opensearch-project/OpenSearch#3542"

This reverts commit 01dd024.

Signed-off-by: Rupanshi Sharma <[email protected]>

Co-authored-by: Rupanshi Sharma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecate enhancement Enhancement or improvement to existing feature or request v2.2.0
Projects
None yet
Development

No branches or pull requests

2 participants