Skip to content

Commit

Permalink
[2.x] Deprecate public class names with master terminology (#3871) (#…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
Tianli Feng authored Jul 15, 2022
1 parent 21c45a5 commit 298cc9c
Show file tree
Hide file tree
Showing 11 changed files with 248 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.cluster;

/**
* Enables listening to cluster-manager changes events of the local node (when the local node becomes the cluster-manager, and when the local
* node cease being a cluster-manager).
*
* @opensearch.internal
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link LocalNodeClusterManagerListener}
*/
@Deprecated
public interface LocalNodeMasterListener extends LocalNodeClusterManagerListener {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.cluster;

import java.util.function.Predicate;

/**
* Utility class to build a predicate that accepts cluster state changes
*
* @opensearch.internal
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link ClusterManagerNodeChangePredicate}
*/
@Deprecated
public final class MasterNodeChangePredicate {

private MasterNodeChangePredicate() {

}

public static Predicate<ClusterState> build(ClusterState currentState) {
return ClusterManagerNodeChangePredicate.build(currentState);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.cluster;

import org.opensearch.common.io.stream.StreamInput;

import java.io.IOException;

/**
* Thrown when a node join request or a cluster-manager ping reaches a node which is not
* currently acting as a cluster-manager or when a cluster state update task is to be executed
* on a node that is no longer cluster-manager.
*
* @opensearch.internal
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link NotClusterManagerException}
*/
@Deprecated
public class NotMasterException extends NotClusterManagerException {

public NotMasterException(String msg) {
super(msg);
}

public NotMasterException(StreamInput in) throws IOException {
super(in);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.cluster.coordination;

import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Settings;

/**
* Service to block the master node
*
* @opensearch.internal
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link NoClusterManagerBlockService}
*/
@Deprecated
public class NoMasterBlockService extends NoClusterManagerBlockService {

public NoMasterBlockService(Settings settings, ClusterSettings clusterSettings) {
super(settings, clusterSettings);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.cluster.coordination;

/**
* Tool to run an unsafe bootstrap
*
* @opensearch.internal
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link UnsafeBootstrapClusterManagerCommand}
*/
@Deprecated
public class UnsafeBootstrapMasterCommand extends UnsafeBootstrapClusterManagerCommand {

UnsafeBootstrapMasterCommand() {
super();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import org.apache.logging.log4j.Logger;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateUpdateTask;
import org.opensearch.cluster.LocalNodeClusterManagerListener;
import org.opensearch.cluster.LocalNodeMasterListener;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.Priority;
Expand Down Expand Up @@ -88,10 +88,10 @@ public ConsistentSettingsService(Settings settings, ClusterService clusterServic
}

/**
* Returns a {@link LocalNodeClusterManagerListener} that will publish hashes of all the settings passed in the constructor. These hashes are
* Returns a {@link LocalNodeMasterListener} that will publish hashes of all the settings passed in the constructor. These hashes are
* published by the cluster-manager node only. Note that this is not designed for {@link SecureSettings} implementations that are mutable.
*/
public LocalNodeClusterManagerListener newHashPublisher() {
public LocalNodeMasterListener newHashPublisher() {
// eagerly compute hashes to be published
final Map<String, String> computedHashesOfConsistentSettings = computeHashesOfConsistentSecureSettings();
return new HashesPublisher(computedHashesOfConsistentSettings, clusterService);
Expand Down Expand Up @@ -246,7 +246,7 @@ private byte[] computeSaltedPBKDF2Hash(byte[] bytes, byte[] salt) {
}
}

static final class HashesPublisher implements LocalNodeClusterManagerListener {
static final class HashesPublisher implements LocalNodeMasterListener {

// eagerly compute hashes to be published
final Map<String, String> computedHashesOfConsistentSettings;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.discovery;

import org.opensearch.common.io.stream.StreamInput;

import java.io.IOException;

/**
* Exception when the cluster-manager is not discovered
*
* @opensearch.internal
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link ClusterManagerNotDiscoveredException}
*/
@Deprecated
public class MasterNotDiscoveredException extends ClusterManagerNotDiscoveredException {

public MasterNotDiscoveredException() {
super();
}

public MasterNotDiscoveredException(Throwable cause) {
super(cause);
}

public MasterNotDiscoveredException(String message) {
super(message);
}

public MasterNotDiscoveredException(StreamInput in) throws IOException {
super(in);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.rest.action.cat;

/**
* _cat API action to list cluster_manager information
*
* @opensearch.api
* @deprecated As of 2.2, because supporting inclusive language, replaced by {@link RestClusterManagerAction}
*/
@Deprecated
public class RestMasterAction extends RestClusterManagerAction {

}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.opensearch.action.search.ShardSearchFailure;
import org.opensearch.action.support.replication.ReplicationOperation;
import org.opensearch.client.AbstractClientHeadersTestCase;
import org.opensearch.cluster.NotMasterException;
import org.opensearch.cluster.action.shard.ShardStateAction;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.coordination.CoordinationStateRejectedException;
Expand All @@ -69,6 +70,7 @@
import org.opensearch.common.util.CancellableThreadsTests;
import org.opensearch.common.util.set.Sets;
import org.opensearch.common.xcontent.XContentLocation;
import org.opensearch.discovery.MasterNotDiscoveredException;
import org.opensearch.env.ShardLockObtainFailedException;
import org.opensearch.index.Index;
import org.opensearch.index.engine.RecoveryEngineException;
Expand Down Expand Up @@ -234,6 +236,9 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
Files.walkFileTree(testStartPath, visitor);
assertTrue(notRegistered.remove(TestException.class));
assertTrue(notRegistered.remove(UnknownHeaderException.class));
// Remove the deprecated exception classes from the unregistered list.
assertTrue(notRegistered.remove(NotMasterException.class));
assertTrue(notRegistered.remove(MasterNotDiscoveredException.class));
assertTrue("Classes subclassing OpenSearchException must be registered \n" + notRegistered.toString(), notRegistered.isEmpty());
assertTrue(registered.removeAll(OpenSearchException.getRegisteredKeys())); // check
assertEquals(registered.toString(), 0, registered.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import org.opensearch.rest.action.admin.cluster.RestGetStoredScriptAction;
import org.opensearch.rest.action.admin.cluster.RestPutStoredScriptAction;
import org.opensearch.rest.action.cat.RestAllocationAction;
import org.opensearch.rest.action.cat.RestMasterAction;
import org.opensearch.rest.action.cat.RestRepositoriesAction;
import org.opensearch.rest.action.cat.RestThreadPoolAction;
import org.opensearch.rest.action.cat.RestClusterManagerAction;
Expand Down Expand Up @@ -139,6 +140,12 @@ public void testCatClusterManager() {
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
}

public void testCatMaster() {
RestMasterAction action = new RestMasterAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.doCatRequest(getRestRequestWithBothParams(), client));
assertThat(e.getMessage(), containsString(DUPLICATE_PARAMETER_ERROR_MESSAGE));
}

public void testCatNodeattrs() {
RestNodeAttrsAction action = new RestNodeAttrsAction();
Exception e = assertThrows(OpenSearchParseException.class, () -> action.doCatRequest(getRestRequestWithBothParams(), client));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateObserver;
import org.opensearch.cluster.LocalNodeClusterManagerListener;
import org.opensearch.cluster.LocalNodeMasterListener;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.coordination.NoClusterManagerBlockService;
import org.opensearch.cluster.metadata.Metadata;
Expand Down Expand Up @@ -331,6 +332,43 @@ public void offMaster() {
timedClusterApplierService.close();
}

/* Validate the backwards compatibility of LocalNodeMasterListener remains
* after making it a subclass of LocalNodeClusterManagerListener.
* Overriding the methods with non-inclusive words are intentional.
* To support inclusive language, LocalNodeMasterListener is deprecated in 2.2.
*/
public void testDeprecatedLocalNodeMasterListenerCallbacks() {
TimedClusterApplierService timedClusterApplierService = createTimedClusterService(false);

AtomicBoolean isClusterManager = new AtomicBoolean();
timedClusterApplierService.addLocalNodeMasterListener(new LocalNodeMasterListener() {
@Override
public void onMaster() {
isClusterManager.set(true);
}

@Override
public void offMaster() {
isClusterManager.set(false);
}
});

ClusterState state = timedClusterApplierService.state();
DiscoveryNodes nodes = state.nodes();
DiscoveryNodes.Builder nodesBuilder = DiscoveryNodes.builder(nodes).masterNodeId(nodes.getLocalNodeId());
state = ClusterState.builder(state).nodes(nodesBuilder).build();
setState(timedClusterApplierService, state);
assertThat(isClusterManager.get(), is(true));

nodes = state.nodes();
nodesBuilder = DiscoveryNodes.builder(nodes).masterNodeId(null);
state = ClusterState.builder(state).nodes(nodesBuilder).build();
setState(timedClusterApplierService, state);
assertThat(isClusterManager.get(), is(false));

timedClusterApplierService.close();
}

public void testClusterStateApplierCantSampleClusterState() throws InterruptedException {
AtomicReference<Throwable> error = new AtomicReference<>();
AtomicBoolean applierCalled = new AtomicBoolean();
Expand Down

0 comments on commit 298cc9c

Please sign in to comment.