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

SOLR-15094: Replace all code references of coreNodeName to replicaName #2234

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.apache.solr.common.util.PropertiesUtil;
import org.apache.solr.core.CoreDescriptor;

import static org.apache.solr.common.params.CommonParams.REPLICA_NAME;

/**
* SolrCloud metadata attached to a {@link CoreDescriptor}.
*/
Expand Down Expand Up @@ -64,7 +66,7 @@ public CloudDescriptor(CoreDescriptor cd, String coreName, Properties props) {
// If no collection name is specified, we default to the core name
this.collectionName = props.getProperty(CoreDescriptor.CORE_COLLECTION, coreName);
this.roles = props.getProperty(CoreDescriptor.CORE_ROLES, null);
this.nodeName = props.getProperty(CoreDescriptor.CORE_NODE_NAME);
this.nodeName = props.getProperty(REPLICA_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename nodeName internally here as well? I think it's a little misleading

if (Strings.isNullOrEmpty(nodeName))
this.nodeName = null;
this.numShards = PropertiesUtil.toInteger(props.getProperty(CloudDescriptor.NUM_SHARDS), null);
Expand Down Expand Up @@ -146,15 +148,27 @@ public Integer getNumShards() {
public void setNumShards(int numShards) {
this.numShards = numShards;
}


@Deprecated
public String getCoreNodeName() {
return nodeName;
}

public String getReplicaName() {
return nodeName;
}

public void setReplicaName(String nodeName) {
this.nodeName = nodeName;
if(nodeName==null) cd.getPersistableStandardProperties().remove(REPLICA_NAME);
else cd.getPersistableStandardProperties().setProperty(REPLICA_NAME, nodeName);
}

@Deprecated
public void setCoreNodeName(String nodeName) {
this.nodeName = nodeName;
if(nodeName==null) cd.getPersistableStandardProperties().remove(CoreDescriptor.CORE_NODE_NAME);
else cd.getPersistableStandardProperties().setProperty(CoreDescriptor.CORE_NODE_NAME, nodeName);
if(nodeName==null) cd.getPersistableStandardProperties().remove(REPLICA_NAME);
else cd.getPersistableStandardProperties().setProperty(REPLICA_NAME, nodeName);
}

public void reload(CloudDescriptor reloadFrom) {
Expand Down
6 changes: 3 additions & 3 deletions solr/core/src/java/org/apache/solr/cloud/CloudUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class CloudUtil {
public static final int DEFAULT_TIMEOUT = 90;

/**
* See if coreNodeName has been taken over by another baseUrl and unload core
* See if replicaName has been taken over by another baseUrl and unload core
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the log on line 80, and the variable name "cnn" as well

* + throw exception if it has been.
*/
public static void checkSharedFSFailoverReplaced(CoreContainer cc, CoreDescriptor desc) {
Expand Down Expand Up @@ -103,12 +103,12 @@ public static void checkSharedFSFailoverReplaced(CoreContainer cc, CoreDescripto
}
}

public static boolean replicaExists(ClusterState clusterState, String collection, String shard, String coreNodeName) {
public static boolean replicaExists(ClusterState clusterState, String collection, String shard, String replicaName) {
DocCollection docCollection = clusterState.getCollectionOrNull(collection);
if (docCollection != null) {
Slice slice = docCollection.getSlice(shard);
if (slice != null) {
return slice.getReplica(coreNodeName) != null;
return slice.getReplica(replicaName) != null;
}
}
return false;
Expand Down
4 changes: 2 additions & 2 deletions solr/core/src/java/org/apache/solr/cloud/ElectionContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ public abstract class ElectionContext implements Closeable {
volatile String leaderSeqPath;
private SolrZkClient zkClient;

public ElectionContext(final String coreNodeName,
public ElectionContext(final String relicaName,
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo :) missing p

final String electionPath, final String leaderPath, final ZkNodeProps leaderProps, final SolrZkClient zkClient) {
assert zkClient != null;
this.id = coreNodeName;
this.id = relicaName;
this.electionPath = electionPath;
this.leaderPath = leaderPath;
this.leaderProps = leaderProps;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ public boolean onTermChanged(ShardTerms terms) {
}

if (solrCore.getCoreDescriptor() == null || solrCore.getCoreDescriptor().getCloudDescriptor() == null) return true;
String coreNodeName = solrCore.getCoreDescriptor().getCloudDescriptor().getCoreNodeName();
if (terms.haveHighestTermValue(coreNodeName)) return true;
if (lastTermDoRecovery.get() < terms.getTerm(coreNodeName)) {
log.info("Start recovery on {} because core's term is less than leader's term", coreNodeName);
lastTermDoRecovery.set(terms.getTerm(coreNodeName));
String replicaName = solrCore.getCoreDescriptor().getCloudDescriptor().getReplicaName();
if (terms.haveHighestTermValue(replicaName)) return true;
if (lastTermDoRecovery.get() < terms.getTerm(replicaName)) {
log.info("Start recovery on {} because core's term is less than leader's term", replicaName);
lastTermDoRecovery.set(terms.getTerm(replicaName));
solrCore.getUpdateHandler().getSolrCoreState().doRecovery(solrCore.getCoreContainer(), solrCore.getCoreDescriptor());
}
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public static interface RecoveryListener {
private RecoveryListener recoveryListener;
private ZkController zkController;
private String baseUrl;
private String coreZkNodeName;
private String replicaName;
private ZkStateReader zkStateReader;
private volatile String coreName;
private int retries;
Expand All @@ -137,7 +137,7 @@ protected RecoveryStrategy(CoreContainer cc, CoreDescriptor cd, RecoveryListener
zkController = cc.getZkController();
zkStateReader = zkController.getZkStateReader();
baseUrl = zkController.getBaseUrl();
coreZkNodeName = cd.getCloudDescriptor().getCoreNodeName();
replicaName = cd.getCloudDescriptor().getCoreNodeName();
Copy link
Contributor

Choose a reason for hiding this comment

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

Update so we're not using deprecated method

replicaType = cd.getCloudDescriptor().getReplicaType();
}

Expand Down Expand Up @@ -193,7 +193,7 @@ final public void close() {
if (prevSendPreRecoveryHttpUriRequest != null) {
prevSendPreRecoveryHttpUriRequest.abort();
}
log.warn("Stopping recovery for core=[{}] coreNodeName=[{}]", coreName, coreZkNodeName);
log.warn("Stopping recovery for core=[{}] replicaName=[{}]", coreName, replicaName);
}

final private void recoveryFailed(final ZkController zkController,
Expand Down Expand Up @@ -847,7 +847,7 @@ final private void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa
WaitForState prepCmd = new WaitForState();
prepCmd.setCoreName(leaderCoreName);
prepCmd.setNodeName(zkController.getNodeName());
prepCmd.setCoreNodeName(coreZkNodeName);
prepCmd.setCoreNodeName(replicaName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Use new method

prepCmd.setState(Replica.State.RECOVERING);
prepCmd.setCheckLive(true);
prepCmd.setOnlyIfLeader(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {

public ShardLeaderElectionContext(LeaderElector leaderElector,
final String shardId, final String collection,
final String coreNodeName, ZkNodeProps props, ZkController zkController, CoreContainer cc) {
super(leaderElector, shardId, collection, coreNodeName, props,
final String replicaName, ZkNodeProps props, ZkController zkController, CoreContainer cc) {
super(leaderElector, shardId, collection, replicaName, props,
zkController);
this.cc = cc;
syncStrategy = new SyncStrategy(cc);
Expand Down Expand Up @@ -133,7 +133,7 @@ void runLeaderProcess(boolean weAreReplacement, int pauseBeforeStart) throws Kee
}

Replica.Type replicaType;
String coreNodeName;
String replicaName;
boolean setTermToMax = false;
try (SolrCore core = cc.getCore(coreName)) {

Expand All @@ -142,11 +142,11 @@ void runLeaderProcess(boolean weAreReplacement, int pauseBeforeStart) throws Kee
}

replicaType = core.getCoreDescriptor().getCloudDescriptor().getReplicaType();
coreNodeName = core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName();
replicaName = core.getCoreDescriptor().getCloudDescriptor().getReplicaName();
// should I be leader?
ZkShardTerms zkShardTerms = zkController.getShardTerms(collection, shardId);
if (zkShardTerms.registered(coreNodeName) && !zkShardTerms.canBecomeLeader(coreNodeName)) {
if (!waitForEligibleBecomeLeaderAfterTimeout(zkShardTerms, coreNodeName, leaderVoteWait)) {
if (zkShardTerms.registered(replicaName) && !zkShardTerms.canBecomeLeader(replicaName)) {
if (!waitForEligibleBecomeLeaderAfterTimeout(zkShardTerms, replicaName, leaderVoteWait)) {
rejoinLeaderElection(core);
return;
} else {
Expand Down Expand Up @@ -255,8 +255,8 @@ void runLeaderProcess(boolean weAreReplacement, int pauseBeforeStart) throws Kee
// in case of leaderVoteWait timeout, a replica with lower term can win the election
if (setTermToMax) {
log.error("WARNING: Potential data loss -- Replica {} became leader after timeout (leaderVoteWait) {}"
, "without being up-to-date with the previous leader", coreNodeName);
zkController.getShardTerms(collection, shardId).setTermEqualsToLeader(coreNodeName);
, "without being up-to-date with the previous leader", replicaName);
zkController.getShardTerms(collection, shardId).setTermEqualsToLeader(replicaName);
}
super.runLeaderProcess(weAreReplacement, 0);
try (SolrCore core = cc.getCore(coreName)) {
Expand Down Expand Up @@ -314,15 +314,15 @@ void runLeaderProcess(boolean weAreReplacement, int pauseBeforeStart) throws Kee
* @return true if after {@code timeout} there are no other replicas with higher term participate in the election,
* false if otherwise
*/
private boolean waitForEligibleBecomeLeaderAfterTimeout(ZkShardTerms zkShardTerms, String coreNodeName, int timeout) throws InterruptedException {
private boolean waitForEligibleBecomeLeaderAfterTimeout(ZkShardTerms zkShardTerms, String replicaName, int timeout) throws InterruptedException {
long timeoutAt = System.nanoTime() + TimeUnit.NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS);
while (!isClosed && !cc.isShutDown()) {
if (System.nanoTime() > timeoutAt) {
log.warn("After waiting for {}ms, no other potential leader was found, {} try to become leader anyway (core_term:{}, highest_term:{})",
timeout, coreNodeName, zkShardTerms.getTerm(coreNodeName), zkShardTerms.getHighestTerm());
timeout, replicaName, zkShardTerms.getTerm(replicaName), zkShardTerms.getHighestTerm());
return true;
}
if (replicasWithHigherTermParticipated(zkShardTerms, coreNodeName)) {
if (replicasWithHigherTermParticipated(zkShardTerms, replicaName)) {
log.info("Can't become leader, other replicas with higher term participated in leader election");
return false;
}
Expand All @@ -336,17 +336,17 @@ private boolean waitForEligibleBecomeLeaderAfterTimeout(ZkShardTerms zkShardTerm
*
* @return true if other replicas with higher term participated in the election, false if otherwise
*/
private boolean replicasWithHigherTermParticipated(ZkShardTerms zkShardTerms, String coreNodeName) {
private boolean replicasWithHigherTermParticipated(ZkShardTerms zkShardTerms, String replicaName) {
ClusterState clusterState = zkController.getClusterState();
DocCollection docCollection = clusterState.getCollectionOrNull(collection);
Slice slices = (docCollection == null) ? null : docCollection.getSlice(shardId);
if (slices == null) return false;

long replicaTerm = zkShardTerms.getTerm(coreNodeName);
boolean isRecovering = zkShardTerms.isRecovering(coreNodeName);
long replicaTerm = zkShardTerms.getTerm(replicaName);
boolean isRecovering = zkShardTerms.isRecovering(replicaName);

for (Replica replica : slices.getReplicas()) {
if (replica.getName().equals(coreNodeName)) continue;
if (replica.getName().equals(replicaName)) continue;

if (clusterState.getLiveNodes().contains(replica.getNodeName())) {
long otherTerm = zkShardTerms.getTerm(replica.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ class ShardLeaderElectionContextBase extends ElectionContext {
private final Object lock = new Object();

public ShardLeaderElectionContextBase(LeaderElector leaderElector,
final String shardId, final String collection, final String coreNodeName,
final String shardId, final String collection, final String replicaName,
ZkNodeProps props, ZkController zkController) {
super(coreNodeName, ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection
super(replicaName, ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection
+ "/leader_elect/" + shardId, ZkStateReader.getShardLeadersPath(
collection, shardId), props, zkController.getZkClient());
this.leaderElector = leaderElector;
Expand Down
4 changes: 2 additions & 2 deletions solr/core/src/java/org/apache/solr/cloud/SyncStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private PeerSync.PeerSyncResult syncReplicas(ZkController zkController, SolrCore
private PeerSync.PeerSyncResult syncWithReplicas(ZkController zkController, SolrCore core,
ZkNodeProps props, String collection, String shardId, boolean peerSyncOnlyWithActive) throws Exception {
List<ZkCoreNodeProps> nodes = zkController.getZkStateReader()
.getReplicaProps(collection, shardId,core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName());
.getReplicaProps(collection, shardId,core.getCoreDescriptor().getCloudDescriptor().getReplicaName());

if (isClosed) {
log.info("We have been closed, won't sync with replicas");
Expand Down Expand Up @@ -197,7 +197,7 @@ private void syncToMe(ZkController zkController, String collection,
List<ZkCoreNodeProps> nodes = zkController
.getZkStateReader()
.getReplicaProps(collection, shardId,
cd.getCloudDescriptor().getCoreNodeName());
cd.getCloudDescriptor().getReplicaName());
if (nodes == null) {
if (log.isInfoEnabled()) {
log.info("{} has no replicas", ZkCoreNodeProps.getCoreUrl(leaderProps));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public ZkShardTerms getShard(String shardId) {
}
}

public void register(String shardId, String coreNodeName) {
public void register(String shardId, String replicaName) {
synchronized (terms) {
getShard(shardId).registerTerm(coreNodeName);
getShard(shardId).registerTerm(replicaName);
}
}

Expand Down
Loading