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 public or protected method and variable that contains "master" terminology in 'server', 'client/rest', 'client/rest-high-level' and 'test/framework' directory #3544

Closed
tlfeng opened this issue Jun 8, 2022 · 3 comments
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 public/protected methods and variables that have "master" in the names. The involved methods and variables are located in server, client/rest, client/rest-high-level and test/framework directories, and the deprecated items will be removed in the next major version (3.0)
  2. Have alternative methods and variables aside, which replaced the non-inclusive name "master" and have the same functionality with the deprecated items.

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

Key points:

  1. Any public/protected method or variable with "master" in the name will directly call the renamed method to maintain only one method/variable while supporting the old ones.

In detail:

  1. Copy the code of method/variable definition and keep it in another place.
  2. Replace master word with clusterManager in the method or variable 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 code of master method/vairable definition back.
  4. Remove all the existing implementation in the old method, and directly call the new renamed method with the same parameters.
  5. Add @Deprecated annotation and @deprecated Javadoc tag for the method/variable that contains "master" in the name.
  6. Backport all the changes to 2.x branch.

Describe alternatives you've considered
None.

Additional context
The regex to filter the lines with public or protected method/variable definition contains "master" terminology:
method: (public|protected)(.)+[Mm]aster(\w)*\(
variable: (public|protected)(.)+(MASTER|master)(.)*=

Totally 195 methods and 21 variables.
The above number need excluding:

  1. There are 15 method definitions and 7 variables that are covered by issue Deprecate package 'org.opensearch.action.support.master' #3542 and Deprecate public or protected class that contains "master" terminology in 'server' and 'test/framework' directory #3543.
  2. 75 masterOperation() overrides which will be resolved by Deprecate package 'org.opensearch.action.support.master' #3542

List of public methods to be renamed:
in sever directory:

public boolean hasDiscoveredMaster() {
public boolean localNodeMaster() {
public void updateMappingOnMaster(Index index, Mapping mappingUpdate, ActionListener<Void> listener) {
public boolean isBecomeMasterTask() {
public Optional<DiscoveryNode> getMasterNode() {
public PutRequest masterTimeout(TimeValue masterTimeout) {
public RemoveRequest masterTimeout(TimeValue masterTimeout) {
public static boolean isMasterNode(Settings settings) {
public boolean isMasterNode() {
public boolean isLocalNodeElectedMaster() {
public ImmutableOpenMap<String, DiscoveryNode> getMasterNodes() {
public ImmutableOpenMap<String, DiscoveryNode> getMasterAndDataNodes() {
public Stream<DiscoveryNode> mastersFirstStream() {
public String getMasterNodeId() {
public DiscoveryNode getMasterNode() {
public boolean masterNodeChanged() {
public DiscoveryNode previousMasterNode() {
public DiscoveryNode newMasterNode() {
public Builder masterNodeId(String clusterManagerNodeId) {
public boolean isLocalNodeElectedMaster() {
protected void assertClusterOrMasterStateThread()
public void addLocalNodeMasterListener(LocalNodeMasterListener listener)
public MasterService getMasterService() (not changed in this PR)
public static boolean assertClusterOrMasterStateThread()
void connectToRemoteMasterNode(TransportAddress transportAddress, ActionListener<DiscoveryNode> listener)
TimeValue masterNodeTimeout()
public synchronized void updateFromMaster()
public ClusterState joinNodesAndBecomeMaster(ClusterState clusterState, List<DiscoveryNode> nodes)

In client directory:

public void setMasterTimeout(TimeValue clusterManagerTimeout)
public TimeValue masterNodeTimeout()
public TimeValue getMasterNodeTimeout()
public void setMasterNodeTimeout(@Nullable TimeValue clusterManagerNodeTimeout)
public void setMasterNodeTimeout(String clusterManagerNodeTimeout)
public boolean isMasterEligible()

In test/framework directory:

public static String blockMasterFromFinalizingSnapshotOnIndexFile(final String repositoryName) {
public static String blockMasterOnWriteIndexFile(final String repositoryName) {
public static void blockMasterFromDeletingIndexNFile(String repositoryName) {
public static String blockMasterFromFinalizingSnapshotOnSnapFile(final String repositoryName) {
public static MasterService createMasterService(ThreadPool threadPool, ClusterState initialClusterState) {
public static MasterService createMasterService(ThreadPool threadPool, DiscoveryNode localNode) {
public abstract int numDataAndMasterNodes();
public Client masterClient() {
public Client nonMasterClient() {
public boolean isMasterEligible() {
public synchronized <T> T getCurrentMasterNodeInstance(Class<T> clazz) {
public <T> Iterable<T> getDataOrMasterNodeInstances(Class<T> clazz) {
public <T> T getMasterNodeInstance(Class<T> clazz) {
public synchronized void stopCurrentMasterNode() throws IOException {
public synchronized void stopRandomNonMasterNode() throws IOException {
public String getMasterName() {
public String getMasterName(@Nullable String viaNode) {
public List<String> startMasterOnlyNodes(int numNodes) {
public List<String> startMasterOnlyNodes(int numNodes, Settings settings) {
public int numMasterNodes() {
public static Settings masterNode()
public static Settings masterNode(final Settings settings)
public static Settings masterOnlyNode()
public static Settings masterOnlyNode(final Settings settings)
public static Settings nonMasterNode()
public static Settings nonMasterNode(final Settings settings)
public Version masterVersion()

List of public variables to be renamed:
In client directory:

public static final TimeValue DEFAULT_MASTER_NODE_TIMEOUT

In test/framework directory:
(all in org.opensearch.test.InternalTestCluster)

public static final int DEFAULT_LOW_NUM_MASTER_NODES
public static final int DEFAULT_HIGH_NUM_MASTER_NODES
public static final int REMOVED_MINIMUM_MASTER_NODES
@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 changed the title Deprecate public or protected method that contains "master" terminology in 'server', 'client/rest', 'client/rest-high-level' and 'test/framework' directory Deprecate public or protected method and variable that contains "master" terminology in 'server', 'client/rest', 'client/rest-high-level' and 'test/framework' directory Jun 9, 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
@tlfeng
Copy link
Collaborator Author

tlfeng commented Jul 20, 2022

Progress update on 07/25/2022:
Completed:
Most public methods and variables in server, client/rest, client/rest-high-level and test/framework are deprecated and renamed, alternatives are created:
public methods and variables in server directory are deprecated and renamed by PR #3647 (backport to 2.x branch in PR #3964).
PR to deal with client directory: #3966 (backport to 2.x in PR #3981)
PR to deal with test/framework directory: #3978 (backport to 2.x in PR #3987)

Remaining:
A few methods need to be deprecated and alternative methods need to be created:
in sever directory:

TimeValue masterNodeTimeout() (in interface AckedRequest)

In test/framework directory:

public static MasterService createMasterService(ThreadPool threadPool, ClusterState initialClusterState) {
public static MasterService createMasterService(ThreadPool threadPool, DiscoveryNode localNode) {
public abstract int numDataAndMasterNodes();

1 Reason that the abstract methods in interface and abstarct class are not renamed:
I plan to deal with them with the other abstract methods masterOperation(), onMaster(), and offMaster() that mentioned in issue #3542 (comment) together, and issue #3543 (comment). They need to be renamed in a special way to keep the backwards compatibility, which already got solution in the above links.

2 Reason that the methods that return MasterService are not renamed:
Blocked by issue #3543.
The class MasterService needs to be deprecated first by issue #3543 (comment), and the 2 methods createMasterService(..) here can be deprecated properly.

@tlfeng
Copy link
Collaborator Author

tlfeng commented Aug 1, 2022

Progress update on 07/31/2022:
Completed:
The 2 methods are deprecated and renamed.

  • In test/framework directory:
    public static MasterService createMasterService(ThreadPool threadPool, ClusterState initialClusterState)
    public static MasterService createMasterService(ThreadPool threadPool, DiscoveryNode localNode)

Remaining:
The abstract methods in the interface will be deprecated with the other one method in issue #3543 (comment) together, using the solution in issue #3688 (comment)

  • In interface AckedRequest: TimeValue masterNodeTimeout() (in interface AckedRequest)

@tlfeng
Copy link
Collaborator Author

tlfeng commented Aug 4, 2022

Progress update on 08/03/2022:
Nothing remains for this issue.
The above remaining item, the abstract method TimeValue masterNodeTimeout() has been deprecated and renamed by PR #4121, and backported to 2.x and 2.2 branches by PR #4122 and #4123.

@tlfeng tlfeng closed this as completed Aug 4, 2022
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

1 participant