lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From va...@apache.org
Subject lucene-solr:master: SOLR-11676: Keep nrtReplicas and replicationFactor in sync while creating a collection and modifying a collection
Date Mon, 18 Jun 2018 08:10:00 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/master 28967e0f6 -> 11fcb2390


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/11fcb239
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/11fcb239
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/11fcb239

Branch: refs/heads/master
Commit: 11fcb23906e8416e74087c5b9d168ef8b777c581
Parents: 28967e0
Author: Varun Thacker <varun@apache.org>
Authored: Mon Jun 18 11:59:28 2018 +0400
Committer: Varun Thacker <varun@apache.org>
Committed: Mon Jun 18 12:09:53 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/11fcb239/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 31dae0b..fc880f5 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -89,6 +89,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/11fcb239/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/11fcb239/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/11fcb239/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/11fcb239/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/11fcb239/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) {


Mime
View raw message