mirror of
https://github.com/apache/lucene.git
synced 2025-02-23 18:55:50 +00:00
SOLR-11676: Keep nrtReplicas and replicationFactor in sync while creating a collection and modifying a collection
This commit is contained in:
parent
28967e0f67
commit
11fcb23906
@ -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
|
||||
----------------------
|
||||
|
||||
|
@ -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));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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)),
|
||||
|
@ -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)) {
|
||||
|
@ -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,
|
||||
|
@ -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) {
|
||||
|
Loading…
x
Reference in New Issue
Block a user