From 01eac2e19d0badf095816fa8f8a546653c0e4556 Mon Sep 17 00:00:00 2001 From: abhidemon Date: Wed, 13 Dec 2017 02:14:56 +0530 Subject: [PATCH 1/4] fix: https://issues.apache.org/jira/browse/SOLR-11624 --- .../solr/cloud/CreateCollectionCmd.java | 21 +++---- .../handler/admin/ConfigSetsHandlerApi.java | 6 ++ .../TimeRoutedAliasUpdateProcessorTest.java | 57 +++++++++++++------ solr/solr-ref-guide/src/collections-api.adoc | 2 +- 4 files changed, 55 insertions(+), 31 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java index 2c4f01edd0fb..5d9eb6461bcb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/CreateCollectionCmd.java @@ -333,10 +333,13 @@ String getConfigName(String coll, ZkNodeProps message) throws KeeperException, I try { configNames = ocmh.zkStateReader.getZkClient().getChildren(ZkConfigManager.CONFIGS_ZKNODE, null, true); if (configNames.contains(ConfigSetsHandlerApi.DEFAULT_CONFIGSET_NAME)) { - if (!CollectionAdminParams.SYSTEM_COLL.equals(coll)) { - copyDefaultConfigSetTo(configNames, coll); + if (CollectionAdminParams.SYSTEM_COLL.equals(coll)) { + return coll; + } else { + String intendedConfigSetName = ConfigSetsHandlerApi.getSuffixedNameForAutoGeneratedConfigSet(coll); + copyDefaultConfigSetTo(configNames, intendedConfigSetName); + return intendedConfigSetName; } - return coll; } else if (configNames != null && configNames.size() == 1) { configName = configNames.get(0); // no config set named, but there is only 1 - use it @@ -355,17 +358,11 @@ String getConfigName(String coll, ZkNodeProps message) throws KeeperException, I private void copyDefaultConfigSetTo(List configNames, String targetConfig) { ZkConfigManager configManager = new ZkConfigManager(ocmh.zkStateReader.getZkClient()); - // if a configset named coll exists, delete the configset so that _default can be copied over + // if a configset named collection exists, re-use it if (configNames.contains(targetConfig)) { log.info("There exists a configset by the same name as the collection we're trying to create: " + targetConfig + - ", deleting it so that we can copy the _default configs over and create the collection."); - try { - configManager.deleteConfigDir(targetConfig); - } catch (Exception e) { - throw new SolrException(ErrorCode.INVALID_STATE, "Error while deleting configset: " + targetConfig, e); - } - } else { - log.info("Only _default config set found, using it."); + ", re-using it."); + return; } // Copy _default into targetConfig try { diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java index 2028f677c416..1239789e99f8 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java @@ -32,10 +32,16 @@ public class ConfigSetsHandlerApi extends BaseHandlerApiSupport { final public static String DEFAULT_CONFIGSET_NAME = "_default"; + final public static String AUTOCREATED_CONFIGSET_SUFFIX = ".AUTOCREATED_CONFIGSET"; + final ConfigSetsHandler configSetHandler; static Collection apiCommands = createMapping(); + public static String getSuffixedNameForAutoGeneratedConfigSet(String configName) { + return configName + AUTOCREATED_CONFIGSET_SUFFIX; + } + private static Collection createMapping() { Map result = new EnumMap<>(ConfigSetMeta.class); diff --git a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java index f7f200f58ae6..a432e7654ce5 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java @@ -32,6 +32,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.ConfigSetAdminRequest; import org.apache.solr.client.solrj.request.V2Request; +import org.apache.solr.client.solrj.response.ConfigSetAdminResponse; import org.apache.solr.client.solrj.response.FieldStatsInfo; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.client.solrj.response.UpdateResponse; @@ -42,6 +43,7 @@ import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.NamedList; +import org.apache.solr.handler.admin.ConfigSetsHandlerApi; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.junit.AfterClass; @@ -73,24 +75,34 @@ public static void finish() throws Exception { @Test public void test() throws Exception { - // First create a config using REST API. To do this, we create a collection with the name of the eventual config. - // We configure it, and ultimately delete it the collection, leaving a config with the same name behind. - // Then when we create the "real" collections referencing this config. + + // First create a configSet + // Then we create a collection with the name of the eventual config. + // We configure it, and ultimately delete it the collection, leaving a modified config-set behind. + // Then when we create the "real" collections referencing this modified config-set. + final ConfigSetAdminRequest.Create adminRequest = new ConfigSetAdminRequest.Create(); + adminRequest.setConfigSetName(configName); + adminRequest.setBaseConfigSetName("_default"); + ConfigSetAdminResponse adminResponse = adminRequest.process(solrClient); + assertEquals(adminResponse.getStatus(), 0); + CollectionAdminRequest.createCollection(configName, 1, 1).process(solrClient); // manipulate the config... + + String conf = "{" + + " 'set-user-property' : {'timePartitionAliasName':'" + alias + "'}," + // no data driven + " 'set-user-property' : {'update.autoCreateFields':false}," + // no data driven + " 'add-updateprocessor' : {" + + " 'name':'tolerant', 'class':'solr.TolerantUpdateProcessorFactory'" + + " }," + + " 'add-updateprocessor' : {" + // for testing + " 'name':'inc', 'class':'" + IncrementURPFactory.class.getName() + "'," + + " 'fieldName':'" + intField + "'" + + " }," + + "}"; checkNoError(solrClient.request(new V2Request.Builder("/collections/" + configName + "/config") .withMethod(SolrRequest.METHOD.POST) - .withPayload("{" + - " 'set-user-property' : {'timePartitionAliasName':'" + alias + "'}," + // no data driven - " 'set-user-property' : {'update.autoCreateFields':false}," + // no data driven - " 'add-updateprocessor' : {" + - " 'name':'tolerant', 'class':'solr.TolerantUpdateProcessorFactory'" + - " }," + - " 'add-updateprocessor' : {" + // for testing - " 'name':'inc', 'class':'" + IncrementURPFactory.class.getName() + "'," + - " 'fieldName':'" + intField + "'" + - " }," + - "}").build())); + .withPayload(conf).build())); checkNoError(solrClient.request(new V2Request.Builder("/collections/" + configName + "/config/params") .withMethod(SolrRequest.METHOD.POST) .withPayload("{" + @@ -100,14 +112,23 @@ public void test() throws Exception { "}").build())); CollectionAdminRequest.deleteCollection(configName).process(solrClient); + String configNameOfModifiedConfigSet = ConfigSetsHandlerApi.getSuffixedNameForAutoGeneratedConfigSet(configName); + assertTrue( + new ConfigSetAdminRequest.List().process(solrClient).getConfigSets() + .contains(configNameOfModifiedConfigSet) + ); + // start with one collection and an alias for it final String col23rd = alias + "_2017-10-23"; - CollectionAdminRequest.createCollection(col23rd, configName, 1, 1) + CollectionAdminRequest.createCollection(col23rd, configNameOfModifiedConfigSet, 1, 1) .withProperty(TimeRoutedAliasUpdateProcessor.TIME_PARTITION_ALIAS_NAME_CORE_PROP, alias) .process(solrClient); - assertEquals("We only expect 2 configSets", - Arrays.asList("_default", configName), new ConfigSetAdminRequest.List().process(solrClient).getConfigSets()); + List retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets(); + List expectedConfigSetNames = Arrays.asList("_default", configName, configNameOfModifiedConfigSet ); + assertTrue("We only expect 2 configSets", + expectedConfigSetNames.size() == retrievedConfigSetNames.size()); + assertTrue("ConfigNames should be :"+expectedConfigSetNames,expectedConfigSetNames.containsAll(retrievedConfigSetNames) && retrievedConfigSetNames.containsAll(expectedConfigSetNames)); CollectionAdminRequest.createAlias(alias, col23rd).process(solrClient); //TODO use SOLR-11617 client API to set alias metadata @@ -134,7 +155,7 @@ public void test() throws Exception { // add another collection, add to alias (soonest comes first) final String col24th = alias + "_2017-10-24"; - CollectionAdminRequest.createCollection(col24th, configName, 2, 2) // more shards and replicas now + CollectionAdminRequest.createCollection(col24th, configNameOfModifiedConfigSet, 2, 2) // more shards and replicas now .setMaxShardsPerNode(2) .withProperty("timePartitionAliasName", alias) .process(solrClient); diff --git a/solr/solr-ref-guide/src/collections-api.adoc b/solr/solr-ref-guide/src/collections-api.adoc index 784e2cf73f6f..37bd9906fbd6 100644 --- a/solr/solr-ref-guide/src/collections-api.adoc +++ b/solr/solr-ref-guide/src/collections-api.adoc @@ -85,7 +85,7 @@ A `false` value makes the results of a collection creation predictable and gives This parameter is ignored if `createNodeSet` is not also specified. `collection.configName`:: -Defines the name of the configurations (which *must already be stored in ZooKeeper*) to use for this collection. If not provided, Solr will default to the collection name as the configuration name. +Defines the name of the configurations (which *must already be stored in ZooKeeper*) to use for this collection. If not provided, Solr will use the configurations of `_default` configSet *OR* `the only configSet present` (if there is only 1 config set in Zookeeper) to create the collection. The new configSet created for the collection will be named `.AUTOCREATED_CONFIGSET`. `router.field`:: If this parameter is specified, the router will look at the value of the field in an input document to compute the hash and identify a shard instead of looking at the `uniqueKey` field. If the field specified is null in the document, the document will be rejected. From aa33eb6d668b65af64b3373aa1920078780a9b6b Mon Sep 17 00:00:00 2001 From: abhidemon Date: Sat, 30 Dec 2017 19:13:04 +0530 Subject: [PATCH 2/4] refactor(defaultConfigSetName): Default config set name --- .../org/apache/solr/handler/admin/ConfigSetsHandlerApi.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java index 1239789e99f8..1a5f6f336219 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandlerApi.java @@ -32,7 +32,7 @@ public class ConfigSetsHandlerApi extends BaseHandlerApiSupport { final public static String DEFAULT_CONFIGSET_NAME = "_default"; - final public static String AUTOCREATED_CONFIGSET_SUFFIX = ".AUTOCREATED_CONFIGSET"; + final public static String AUTOCREATED_CONFIGSET_SUFFIX = ".AUTOCREATED"; final ConfigSetsHandler configSetHandler; From f98f49a856029f850482ebf1e626102c44da27b3 Mon Sep 17 00:00:00 2001 From: abhidemon Date: Sat, 30 Dec 2017 19:14:22 +0530 Subject: [PATCH 3/4] test(TimeRoutedAliasUP): Instead of using default name of the custom collection, using an explicit name. --- .../TimeRoutedAliasUpdateProcessorTest.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java index a432e7654ce5..fae9bd0e513e 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/TimeRoutedAliasUpdateProcessorTest.java @@ -78,7 +78,7 @@ public void test() throws Exception { // First create a configSet // Then we create a collection with the name of the eventual config. - // We configure it, and ultimately delete it the collection, leaving a modified config-set behind. + // We configure it, and ultimately delete the collection, leaving a modified config-set behind. // Then when we create the "real" collections referencing this modified config-set. final ConfigSetAdminRequest.Create adminRequest = new ConfigSetAdminRequest.Create(); adminRequest.setConfigSetName(configName); @@ -86,7 +86,7 @@ public void test() throws Exception { ConfigSetAdminResponse adminResponse = adminRequest.process(solrClient); assertEquals(adminResponse.getStatus(), 0); - CollectionAdminRequest.createCollection(configName, 1, 1).process(solrClient); + CollectionAdminRequest.createCollection(configName, configName,1, 1).process(solrClient); // manipulate the config... String conf = "{" + @@ -112,20 +112,19 @@ public void test() throws Exception { "}").build())); CollectionAdminRequest.deleteCollection(configName).process(solrClient); - String configNameOfModifiedConfigSet = ConfigSetsHandlerApi.getSuffixedNameForAutoGeneratedConfigSet(configName); assertTrue( new ConfigSetAdminRequest.List().process(solrClient).getConfigSets() - .contains(configNameOfModifiedConfigSet) + .contains(configName) ); // start with one collection and an alias for it final String col23rd = alias + "_2017-10-23"; - CollectionAdminRequest.createCollection(col23rd, configNameOfModifiedConfigSet, 1, 1) + CollectionAdminRequest.createCollection(col23rd, configName, 1, 1) .withProperty(TimeRoutedAliasUpdateProcessor.TIME_PARTITION_ALIAS_NAME_CORE_PROP, alias) .process(solrClient); List retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets(); - List expectedConfigSetNames = Arrays.asList("_default", configName, configNameOfModifiedConfigSet ); + List expectedConfigSetNames = Arrays.asList("_default", configName); assertTrue("We only expect 2 configSets", expectedConfigSetNames.size() == retrievedConfigSetNames.size()); assertTrue("ConfigNames should be :"+expectedConfigSetNames,expectedConfigSetNames.containsAll(retrievedConfigSetNames) && retrievedConfigSetNames.containsAll(expectedConfigSetNames)); @@ -155,7 +154,7 @@ public void test() throws Exception { // add another collection, add to alias (soonest comes first) final String col24th = alias + "_2017-10-24"; - CollectionAdminRequest.createCollection(col24th, configNameOfModifiedConfigSet, 2, 2) // more shards and replicas now + CollectionAdminRequest.createCollection(col24th, configName, 2, 2) // more shards and replicas now .setMaxShardsPerNode(2) .withProperty("timePartitionAliasName", alias) .process(solrClient); From 8804dc8d0168e3e3b9219d16377a096a40d21dd2 Mon Sep 17 00:00:00 2001 From: abhidemon Date: Sat, 30 Dec 2017 19:50:44 +0530 Subject: [PATCH 4/4] docs(create-collection-api): --- solr/solr-ref-guide/src/collections-api.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/solr-ref-guide/src/collections-api.adoc b/solr/solr-ref-guide/src/collections-api.adoc index 37bd9906fbd6..1222ee01be88 100644 --- a/solr/solr-ref-guide/src/collections-api.adoc +++ b/solr/solr-ref-guide/src/collections-api.adoc @@ -85,7 +85,7 @@ A `false` value makes the results of a collection creation predictable and gives This parameter is ignored if `createNodeSet` is not also specified. `collection.configName`:: -Defines the name of the configurations (which *must already be stored in ZooKeeper*) to use for this collection. If not provided, Solr will use the configurations of `_default` configSet *OR* `the only configSet present` (if there is only 1 config set in Zookeeper) to create the collection. The new configSet created for the collection will be named `.AUTOCREATED_CONFIGSET`. +Defines the name of the configuration (which *must already be stored in ZooKeeper*) to use for this collection. If not provided, Solr will use the configuration of `_default` configSet *OR* `the only configSet present` (if there is only 1 config set in Zookeeper) to create a new (and mutable) configSet named `.AUTOCREATED` and will use it for the new collection. `router.field`:: If this parameter is specified, the router will look at the value of the field in an input document to compute the hash and identify a shard instead of looking at the `uniqueKey` field. If the field specified is null in the document, the document will be rejected.