You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by va...@apache.org on 2018/06/18 08:23:31 UTC
lucene-solr:branch_7x: SOLR-11676: Keep nrtReplicas and
replicationFactor in sync while creating a collection and modifying a
collection
Repository: lucene-solr
Updated Branches:
refs/heads/branch_7x e5407c5a9 -> de71d48e0
SOLR-11676: Keep nrtReplicas and replicationFactor in sync while creating a collection and modifying a collection
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/de71d48e
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/de71d48e
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/de71d48e
Branch: refs/heads/branch_7x
Commit: de71d48e0afa1e9829f14731268554b29d0d5823
Parents: e5407c5
Author: Varun Thacker <va...@apache.org>
Authored: Mon Jun 18 11:59:28 2018 +0400
Committer: Varun Thacker <va...@apache.org>
Committed: Mon Jun 18 12:12:12 2018 +0400
----------------------------------------------------------------------
solr/CHANGES.txt | 4 ++
.../solr/cloud/overseer/CollectionMutator.java | 7 ++-
.../solr/handler/admin/CollectionsHandler.java | 19 ++++++
.../api/collections/TestCollectionAPI.java | 61 ++++++++++++++++++++
.../solr/handler/admin/TestCollectionAPIs.java | 8 +--
.../solrj/request/CollectionAdminRequest.java | 1 -
6 files changed, 94 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/de71d48e/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 2aef443..da4833b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -60,6 +60,10 @@ Bug Fixes
* SOLR-12449: Response /autoscaling/diagnostics shows improper json (noble)
+* SOLR-11676: Keep nrtReplicas and replicationFactor in sync while creating a collection and modifying a collection
+ (Varun Thacker)
+
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/de71d48e/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java b/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
index 05ae012..5ce537f 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
@@ -38,6 +38,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS;
+import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR;
import static org.apache.solr.common.params.CommonParams.NAME;
public class CollectionMutator {
@@ -106,9 +108,12 @@ public class CollectionMutator {
Map<String, Object> m = coll.shallowCopy();
boolean hasAnyOps = false;
for (String prop : CollectionsHandler.MODIFIABLE_COLL_PROPS) {
- if(message.get(prop)!= null) {
+ if (message.get(prop) != null) {
hasAnyOps = true;
m.put(prop,message.get(prop));
+ if (prop == REPLICATION_FACTOR) { //SOLR-11676 : keep NRT_REPLICAS and REPLICATION_FACTOR in sync
+ m.put(NRT_REPLICAS, message.get(REPLICATION_FACTOR));
+ }
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/de71d48e/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index 269bb50..9c45a01 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -484,6 +484,22 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
if (props.get(STATE_FORMAT) == null) {
props.put(STATE_FORMAT, "2");
}
+
+ if (props.get(REPLICATION_FACTOR) != null && props.get(NRT_REPLICAS) != null) {
+ //TODO: Remove this in 8.0 . Keep this for SolrJ client back-compat. See SOLR-11676 for more details
+ int replicationFactor = Integer.parseInt((String) props.get(REPLICATION_FACTOR));
+ int nrtReplicas = Integer.parseInt((String) props.get(NRT_REPLICAS));
+ if (replicationFactor != nrtReplicas) {
+ throw new SolrException(ErrorCode.BAD_REQUEST,
+ "Cannot specify both replicationFactor and nrtReplicas as they mean the same thing");
+ }
+ }
+ if (props.get(REPLICATION_FACTOR) != null) {
+ props.put(NRT_REPLICAS, props.get(REPLICATION_FACTOR));
+ } else if (props.get(NRT_REPLICAS) != null) {
+ props.put(REPLICATION_FACTOR, props.get(NRT_REPLICAS));
+ }
+
addMapObject(props, RULE);
addMapObject(props, SNITCH);
verifyRuleParams(h.coreContainer, props);
@@ -907,6 +923,9 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
addMapObject(m, SNITCH);
for (String prop : MODIFIABLE_COLL_PROPS) DocCollection.verifyProp(m, prop);
verifyRuleParams(h.coreContainer, m);
+ if (m.get(REPLICATION_FACTOR) != null) {
+ m.put(NRT_REPLICAS, m.get(REPLICATION_FACTOR));
+ }
return m;
}),
MIGRATESTATEFORMAT_OP(MIGRATESTATEFORMAT, (req, rsp, h) -> copy(req.getParams().required(), null, COLLECTION_PROP)),
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/de71d48e/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
index d9ea73c..40047f7 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
@@ -92,11 +92,72 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
clusterStatusZNodeVersion();
testClusterStateMigration();
testCollectionCreationCollectionNameValidation();
+ testReplicationFactorValidaton();
testCollectionCreationShardNameValidation();
testAliasCreationNameValidation();
testShardCreationNameValidation();
}
+ private void testReplicationFactorValidaton() throws Exception {
+ try (CloudSolrClient client = createCloudClient(null)) {
+ //Test that you can't specify both replicationFactor and nrtReplicas
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("action", CollectionParams.CollectionAction.CREATE.toString());
+ params.set("name", "test_repFactorColl");
+ params.set("numShards", "1");
+ params.set("replicationFactor", "1");
+ params.set("nrtReplicas", "2");
+ SolrRequest request = new QueryRequest(params);
+ request.setPath("/admin/collections");
+
+ try {
+ client.request(request);
+ fail();
+ } catch (RemoteSolrException e) {
+ final String errorMessage = e.getMessage();
+ assertTrue(errorMessage.contains("Cannot specify both replicationFactor and nrtReplicas as they mean the same thing"));
+ }
+
+ //Create it again correctly
+ CollectionAdminRequest.Create req = CollectionAdminRequest.createCollection("test_repFactorColl", "conf1", 1, 3, 0, 0);
+ client.request(req);
+
+ waitForCollection(cloudClient.getZkStateReader(), "test_repFactorColl", 1);
+ waitForRecoveriesToFinish("test_repFactorColl", false);
+
+ //Assert that replicationFactor has also been set to 3
+ assertCountsForRepFactorAndNrtReplicas(client, "test_repFactorColl");
+
+ params = new ModifiableSolrParams();
+ params.set("action", CollectionParams.CollectionAction.MODIFYCOLLECTION.toString());
+ params.set("collection", "test_repFactorColl");
+ params.set("replicationFactor", "4");
+ request = new QueryRequest(params);
+ request.setPath("/admin/collections");
+ client.request(request);
+
+ assertCountsForRepFactorAndNrtReplicas(client, "test_repFactorColl");
+ }
+ }
+
+ private void assertCountsForRepFactorAndNrtReplicas(CloudSolrClient client, String collectionName) throws Exception {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("action", CollectionParams.CollectionAction.CLUSTERSTATUS.toString());
+ params.set("collection", collectionName);
+ QueryRequest request = new QueryRequest(params);
+ request.setPath("/admin/collections");
+
+ NamedList<Object> rsp = client.request(request);
+ NamedList<Object> cluster = (NamedList<Object>) rsp.get("cluster");
+ assertNotNull("Cluster state should not be null", cluster);
+ NamedList<Object> collections = (NamedList<Object>) cluster.get("collections");
+ assertNotNull("Collections should not be null in cluster state", collections);
+ assertEquals(1, collections.size());
+ Map<String, Object> collection = (Map<String, Object>) collections.get(collectionName);
+ assertNotNull(collection);
+ assertEquals(collection.get("replicationFactor"), collection.get("nrtReplicas"));
+ }
+
private void clusterStatusWithCollectionAndShard() throws IOException, SolrServerException {
try (CloudSolrClient client = createCloudClient(null)) {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/de71d48e/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java b/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
index 0ac81a8..aa3bac6 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java
@@ -88,20 +88,20 @@ public class TestCollectionAPIs extends SolrTestCaseJ4 {
//test a simple create collection call
compareOutput(apiBag, "/collections", POST,
"{create:{name:'newcoll', config:'schemaless', numShards:2, replicationFactor:2 }}", null,
- "{name:newcoll, fromApi:'true', replicationFactor:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create}");
+ "{name:newcoll, fromApi:'true', replicationFactor:'2', nrtReplicas:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create}");
compareOutput(apiBag, "/collections", POST,
"{create:{name:'newcoll', config:'schemaless', numShards:2, nrtReplicas:2 }}", null,
- "{name:newcoll, fromApi:'true', nrtReplicas:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create}");
+ "{name:newcoll, fromApi:'true', nrtReplicas:'2', replicationFactor:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create}");
compareOutput(apiBag, "/collections", POST,
"{create:{name:'newcoll', config:'schemaless', numShards:2, nrtReplicas:2, tlogReplicas:2, pullReplicas:2 }}", null,
- "{name:newcoll, fromApi:'true', nrtReplicas:'2', tlogReplicas:'2', pullReplicas:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create}");
+ "{name:newcoll, fromApi:'true', nrtReplicas:'2', replicationFactor:'2', tlogReplicas:'2', pullReplicas:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create}");
//test a create collection with custom properties
compareOutput(apiBag, "/collections", POST,
"{create:{name:'newcoll', config:'schemaless', numShards:2, replicationFactor:2, properties:{prop1:'prop1val', prop2: prop2val} }}", null,
- "{name:newcoll, fromApi:'true', replicationFactor:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create, property.prop1:prop1val, property.prop2:prop2val}");
+ "{name:newcoll, fromApi:'true', replicationFactor:'2', nrtReplicas:'2', collection.configName:schemaless, numShards:'2', stateFormat:'2', operation:create, property.prop1:prop1val, property.prop2:prop2val}");
compareOutput(apiBag, "/collections", POST,
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/de71d48e/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
index 4566033..e3e9196 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
@@ -508,7 +508,6 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
params.set("router.field", routerField);
}
if (nrtReplicas != null) {
- params.set( ZkStateReader.REPLICATION_FACTOR, nrtReplicas);// Keep both for compatibility?
params.set( ZkStateReader.NRT_REPLICAS, nrtReplicas);
}
if (autoAddReplicas != null) {