This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 9ae053218b9 SOLR-17564 Assign.java: remove old back-compat logic
(#2871)
9ae053218b9 is described below
commit 9ae053218b9398fb5335a9b398cc359ceb10c8f7
Author: pjmcarthur <[email protected]>
AuthorDate: Fri Nov 22 13:04:22 2024 -0800
SOLR-17564 Assign.java: remove old back-compat logic (#2871)
Remove code in Assign.java that was required for compatibility with
collections created before 7.0
Assign is part of SolrCloud's replica naming and node assignment logic.
Co-authored-by: pmcarthur-apache <[email protected]>
---
solr/CHANGES.txt | 2 +
.../apache/solr/cloud/api/collections/Assign.java | 77 ++-----------
.../cloud/api/collections/CreateCollectionCmd.java | 4 +-
.../cloud/AssignBackwardCompatibilityTest.java | 126 ---------------------
4 files changed, 12 insertions(+), 197 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 1ddedd3ec42..28660398656 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -91,6 +91,8 @@ Deprecation Removals
* SOLR-17256: Previously deprecated `SolrRequest` methods `setBasePath` and
`getBasePath` have been removed. SolrJ users
wishing to temporarily override an HTTP client's base URL may use
`Http2SolrClient.requestWithBaseUrl` instead. (Jason Gerlowski)
+* SOLR-17564: Remove code in Assign used for backwards compatibility with
Collections created prior to 7.0 (Paul McArthur)
+
Dependency Upgrades
---------------------
(No changes)
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
index 1d7d3a4087d..13caeea36ec 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
@@ -17,7 +17,6 @@
package org.apache.solr.cloud.api.collections;
import static
org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET;
-import static org.apache.solr.common.cloud.ZkStateReader.CORE_NAME_PROP;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
@@ -70,8 +69,7 @@ public class Assign {
return ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection + "/counter";
}
- public static int incAndGetId(
- DistribStateManager stateManager, String collection, int defaultValue) {
+ public static int incAndGetId(DistribStateManager stateManager, String
collection) {
String path = ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection;
try {
if (!stateManager.hasData(path)) {
@@ -84,8 +82,7 @@ public class Assign {
path += "/counter";
if (!stateManager.hasData(path)) {
try {
- stateManager.createData(
- path, NumberUtils.intToBytes(defaultValue),
CreateMode.PERSISTENT);
+ stateManager.createData(path, NumberUtils.intToBytes(0),
CreateMode.PERSISTENT);
} catch (AlreadyExistsException e) {
// it's okay if another beats us creating the node
}
@@ -116,7 +113,7 @@ public class Assign {
stateManager.setData(path, bytes, version);
return currentId;
} catch (BadVersionException e) {
- continue;
+ // Outdated version, try again
} catch (IOException | KeeperException e) {
throw new SolrException(
SolrException.ErrorCode.SERVER_ERROR,
@@ -134,16 +131,7 @@ public class Assign {
public static String assignCoreNodeName(
DistribStateManager stateManager, DocCollection collection) {
- // for backward compatibility;
- int defaultValue = defaultCounterValue(collection, false);
- String coreNodeName =
- "core_node" + incAndGetId(stateManager, collection.getName(),
defaultValue);
- while (collection.getReplica(coreNodeName) != null) {
- // there is wee chance that, the new coreNodeName id not totally unique,
- // but this will be guaranteed unique for new collections
- coreNodeName = "core_node" + incAndGetId(stateManager,
collection.getName(), defaultValue);
- }
- return coreNodeName;
+ return "core_node" + incAndGetId(stateManager, collection.getName());
}
/**
@@ -204,63 +192,16 @@ public class Assign {
replicaNum);
}
- private static int defaultCounterValue(
- DocCollection collection, boolean newCollection, String shard) {
- if (newCollection || collection == null) return 0;
-
- int defaultValue;
- if (collection.getSlice(shard) != null &&
collection.getSlice(shard).getReplicas().isEmpty()) {
- return 0;
- } else {
- defaultValue = collection.getReplicas().size() * 2;
- }
-
- if (collection.getReplicationFactor() != null) {
- // numReplicas and replicationFactor * numSlices can be not equals,
- // in case of many addReplicas or deleteReplicas are executed
- defaultValue =
- Math.max(defaultValue, collection.getReplicationFactor() *
collection.getSlices().size());
- }
- return defaultValue;
- }
-
- private static int defaultCounterValue(DocCollection collection, boolean
newCollection) {
- if (newCollection) return 0;
- int defaultValue = collection.getReplicas().size();
- return defaultValue;
- }
-
public static String buildSolrCoreName(
- DistribStateManager stateManager,
- String collectionName,
- DocCollection collection,
- String shard,
- Replica.Type type,
- boolean newCollection) {
-
- int defaultValue = defaultCounterValue(collection, newCollection, shard);
- int replicaNum = incAndGetId(stateManager, collectionName, defaultValue);
- String coreName = buildSolrCoreName(collectionName, shard, type,
replicaNum);
- while (collection != null && existCoreName(coreName,
collection.getSlice(shard))) {
- replicaNum = incAndGetId(stateManager, collectionName, defaultValue);
- coreName = buildSolrCoreName(collectionName, shard, type, replicaNum);
- }
- return coreName;
+ DistribStateManager stateManager, String collectionName, String shard,
Replica.Type type) {
+
+ int replicaNum = incAndGetId(stateManager, collectionName);
+ return buildSolrCoreName(collectionName, shard, type, replicaNum);
}
public static String buildSolrCoreName(
DistribStateManager stateManager, DocCollection collection, String
shard, Replica.Type type) {
- return buildSolrCoreName(stateManager, collection.getName(), collection,
shard, type, false);
- }
-
- private static boolean existCoreName(String coreName, Slice slice) {
- if (slice == null) return false;
- for (Replica replica : slice.getReplicas()) {
- if (coreName.equals(replica.getStr(CORE_NAME_PROP))) {
- return true;
- }
- }
- return false;
+ return buildSolrCoreName(stateManager, collection.getName(), shard, type);
}
public static List<String> getLiveOrLiveAndCreateNodeSetList(
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index 5592303580a..2b8a5007bf1 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -293,10 +293,8 @@ public class CreateCollectionCmd implements
CollApiCmds.CollectionApiCommand {
Assign.buildSolrCoreName(
ccc.getSolrCloudManager().getDistribStateManager(),
collectionName,
-
ccc.getSolrCloudManager().getClusterState().getCollectionOrNull(collectionName),
replicaPosition.shard,
- replicaPosition.type,
- true);
+ replicaPosition.type);
if (log.isDebugEnabled()) {
log.debug(
formatString(
diff --git
a/solr/core/src/test/org/apache/solr/cloud/AssignBackwardCompatibilityTest.java
b/solr/core/src/test/org/apache/solr/cloud/AssignBackwardCompatibilityTest.java
deleted file mode 100644
index 0587fada196..00000000000
---
a/solr/core/src/test/org/apache/solr/cloud/AssignBackwardCompatibilityTest.java
+++ /dev/null
@@ -1,126 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.solr.cloud;
-
-import java.io.IOException;
-import java.lang.invoke.MethodHandles;
-import java.util.HashSet;
-import java.util.Set;
-import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.request.CollectionAdminRequest;
-import org.apache.solr.client.solrj.response.CollectionAdminResponse;
-import org.apache.solr.common.cloud.DocCollection;
-import org.apache.solr.common.cloud.Replica;
-import org.apache.solr.util.NumberUtils;
-import org.apache.zookeeper.KeeperException;
-import org.apache.zookeeper.data.Stat;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-// TODO Remove in Solr 9.0
-/**
- * Test for backward compatibility when users update from 6.x or 7.0 to 7.1,
then the counter of
- * collection does not exist in Zk
- */
-public class AssignBackwardCompatibilityTest extends SolrCloudTestCase {
- private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-
- private static final String COLLECTION = "collection1";
-
- @BeforeClass
- public static void setupCluster() throws Exception {
- configureCluster(4)
- .addConfig(
- "conf1",
TEST_PATH().resolve("configsets").resolve("cloud-dynamic").resolve("conf"))
- .configure();
- CollectionAdminRequest.createCollection(COLLECTION, 1,
4).process(cluster.getSolrClient());
- cluster.waitForActiveCollection(COLLECTION, 1, 4);
- }
-
- @Test
- public void test()
- throws IOException, SolrServerException, KeeperException,
InterruptedException {
- Set<String> coreNames = new HashSet<>();
- Set<String> coreNodeNames = new HashSet<>();
-
- int numOperations = random().nextInt(15) + 15;
- int numLiveReplicas = 4;
-
- boolean clearedCounter = false;
- for (int i = 0; i < numOperations; i++) {
- if (log.isInfoEnabled()) {
- log.info("Collection counter={} i={}", getCounter(), i);
- }
- boolean deleteReplica = random().nextBoolean() && numLiveReplicas > 1;
- // No need to clear counter more than one time
- if (random().nextBoolean() && i > 5 && !clearedCounter) {
- log.info("Clear collection counter");
- // clear counter
- cluster.getZkClient().delete("/collections/" + COLLECTION +
"/counter", -1, true);
- clearedCounter = true;
- }
- if (deleteReplica) {
- cluster.waitForActiveCollection(COLLECTION, 1, numLiveReplicas);
- DocCollection dc = getCollectionState(COLLECTION);
- Replica replica =
- getRandomReplica(dc.getSlice("shard1"), (r) -> r.getState() ==
Replica.State.ACTIVE);
- CollectionAdminRequest.deleteReplica(COLLECTION, "shard1",
replica.getName())
- .process(cluster.getSolrClient());
- coreNames.remove(replica.getCoreName());
- numLiveReplicas--;
- } else {
- CollectionAdminResponse response =
- CollectionAdminRequest.addReplicaToShard(COLLECTION, "shard1")
- .process(cluster.getSolrClient());
- assertTrue(response.isSuccess());
- String coreName =
response.getCollectionCoresStatus().keySet().iterator().next();
- assertFalse(
- "Core name is not unique coreName=" + coreName + " " + coreNames,
- coreNames.contains(coreName));
- coreNames.add(coreName);
- numLiveReplicas++;
- cluster.waitForActiveCollection(COLLECTION, 1, numLiveReplicas);
-
- Replica newReplica =
- getCollectionState(COLLECTION).getReplicas().stream()
- .filter(r -> r.getCoreName().equals(coreName))
- .findAny()
- .get();
- String coreNodeName = newReplica.getName();
- assertFalse("Core node name is not unique",
coreNodeNames.contains(coreName));
- coreNodeNames.add(coreNodeName);
- }
- }
- }
-
- private int getCounter() throws InterruptedException {
- try {
- byte[] data =
- cluster
- .getZkClient()
- .getData("/collections/" + COLLECTION + "/counter", null, new
Stat(), true);
- int count = NumberUtils.bytesToInt(data);
- if (count < 0) throw new AssertionError("Found negative collection
counter " + count);
- return count;
- } catch (KeeperException e) {
- return -1;
- }
- }
-}
