You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by gu...@apache.org on 2019/02/19 14:53:37 UTC

[lucene-solr] branch solr-13131 updated (4168f0b -> 01737e1)

This is an automated email from the ASF dual-hosted git repository.

gus pushed a change to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from 4168f0b  SOLR-13149 Tests, fix deadlock exposed by test
     new e142e85  SOLR-13150: first version
     new f1ed575  SOLR-13150: improve assertions for CRA max cardinality test
     new 1208d69  SOLR-13150: fix CRA max cardinality test compatiblity with TolerantUpdateProcessor
     new 3541516  SOLR-13150: throw exception if not all required params were supplied
     new 5b636f6  SOLR-13150: ensure required params are supplied only on createalias
     new 31cacfd  SOLR-13150 fix tests moshe didn't have access to and remove my print statements from previous commit.
     new cc1e108  SOLR-13150 couple minor tweaks
     new 326d8c9  SOLR-13150 precommit fixes
     new 01737e1  SOLR-13150 add some javadoc

The 9 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../cloud/api/collections/CategoryRoutedAlias.java | 42 ++++++++--
 .../solr/cloud/api/collections/CreateAliasCmd.java |  6 ++
 .../solr/handler/admin/CollectionsHandler.java     | 16 ++--
 .../CategoryRoutedAliasUpdateProcessorTest.java    | 96 +++++++++++++++++++---
 .../solrj/request/CollectionAdminRequest.java      | 11 +++
 5 files changed, 146 insertions(+), 25 deletions(-)


[lucene-solr] 05/09: SOLR-13150: ensure required params are supplied only on createalias

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 5b636f636425ae6f4e36ab250f28655b44732809
Author: internet <in...@localhost.localdomain>
AuthorDate: Mon Feb 18 16:27:48 2019 +0200

    SOLR-13150: ensure required params are supplied only on createalias
---
 .../cloud/api/collections/CategoryRoutedAlias.java |  3 ++-
 .../solr/cloud/api/collections/CreateAliasCmd.java |  6 ++++++
 .../solr/handler/admin/CollectionsHandler.java     |  4 ----
 .../CategoryRoutedAliasUpdateProcessorTest.java    | 24 +++++++++++++++++++++-
 4 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index 88b68ea..0c98562 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -128,7 +128,8 @@ public class CategoryRoutedAlias implements RoutedAlias {
 
     if (cols.stream()
         .filter(x -> !x.contains(UNINITIALIZED)).count() >= Integer.valueOf(maxCardinality)) {
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "max cardinality can not be exceeded for a Category Routed Alias: " + maxCardinality);
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Max cardinality " + maxCardinality
+          + " reached for Category Routed Alias: " + getAliasName());
     }
   }
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
index 0e41baf..641c4ad 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java
@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import com.google.common.collect.Sets;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.ZkNodeProps;
@@ -117,6 +118,11 @@ public class CreateAliasCmd extends AliasCmd {
       throw new SolrException(SERVER_ERROR,"Tried to create a routed alias with no type!");
     }
 
+    if (!props.keySet().containsAll(routedAlias.getRequiredParams())) {
+      throw new SolrException(BAD_REQUEST, "Not all required params were supplied. Missing params: " +
+          StrUtils.join(Sets.difference(routedAlias.getRequiredParams(), props.keySet()), ','));
+    }
+
     // Create the first collection.
     String initialColl = routedAlias.computeInitialCollectionName();
       ensureAliasCollection(aliasName, zkStateReader, state, routedAlias.getAliasMetadata(), initialColl);
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 293f731..f1f9a27 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
@@ -584,10 +584,6 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
 
       // Now filter out just the parameters we care about from the request
       Map<String, Object> result = copy(req.getParams(), null, routedAlias.getRequiredParams());
-      if (!result.keySet().containsAll(routedAlias.getRequiredParams())) {
-        throw new SolrException(BAD_REQUEST, "Not all required params were supplied. Missing params: " +
-            StrUtils.join(Sets.difference(routedAlias.getRequiredParams(), result.keySet()), ','));
-      }
       copy(req.getParams(), result, routedAlias.getOptionalParams());
 
       ModifiableSolrParams createCollParams = new ModifiableSolrParams(); // without prefix
diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index ee7fbb5..8607a52 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -221,6 +221,28 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
   }
   @Slow
   @Test
+  public void testMissingRequiredParams() throws Exception {
+    String configName = getSaferTestName();
+    createConfigSet(configName);
+
+    List<String> retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets();
+    List<String> expectedConfigSetNames = Arrays.asList("_default", configName);
+
+    // config sets leak between tests so we can't be any more specific than this on the next 2 asserts
+    assertTrue("We expect at least 2 configSets",
+        retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
+    assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
+
+    SolrException e = expectThrows(SolrException.class, () ->CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+        CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
+            .setMaxShardsPerNode(2))
+        .process(solrClient)
+    );
+    assertTrue("Create Alias should fail since not all required params were supplied", e.getMessage().contains("Not all required params were supplied"));
+  }
+
+  @Slow
+  @Test
   public void testMaxCardinality() throws Exception {
     String configName = getSaferTestName();
     createConfigSet(configName);
@@ -259,7 +281,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     assertInvariants(colVogon, colHoG);
 
     // should fail since max cardinality is reached
-    testFailedDocument(newDoc(SHIPS[2]), "max cardinality can not be exceeded for a Category Routed Alias");
+    testFailedDocument(newDoc(SHIPS[2]), "Max cardinality");
     assertInvariants(colVogon, colHoG);
   }
 


[lucene-solr] 07/09: SOLR-13150 couple minor tweaks

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit cc1e108b2041d1e7b18ffbc069b7d9aef82b1c70
Author: Gus Heck <gu...@apache.org>
AuthorDate: Mon Feb 18 23:31:23 2019 -0500

    SOLR-13150 couple minor tweaks
---
 .../solr/cloud/api/collections/CategoryRoutedAlias.java  |  2 +-
 .../apache/solr/handler/admin/CollectionsHandler.java    | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index 0c98562..a5a5bce 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -128,7 +128,7 @@ public class CategoryRoutedAlias implements RoutedAlias {
 
     if (cols.stream()
         .filter(x -> !x.contains(UNINITIALIZED)).count() >= Integer.valueOf(maxCardinality)) {
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Max cardinality " + maxCardinality
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Max cardinality " + maxCardinality
           + " reached for Category Routed Alias: " + getAliasName());
     }
   }
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 f1f9a27..c448c71 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
@@ -564,7 +564,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
       }
       if (collections != null) {
         if (routedAlias != null) {
-          throw new SolrException(BAD_REQUEST, "Collections cannot be specified when creating a time routed alias.");
+          throw new SolrException(BAD_REQUEST, "Collections cannot be specified when creating a routed alias.");
         } else {
           //////////////////////////////////////
           // Regular alias creation indicated //
@@ -1277,14 +1277,14 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
       log.info("Not waiting for active collection due to exception: " + createCollResponse.getResponse().get("exception"));
       return;
     }
-    
+
     int replicaFailCount;
     if (createCollResponse.getResponse().get("failure") != null) {
       replicaFailCount = ((NamedList) createCollResponse.getResponse().get("failure")).size();
     } else {
       replicaFailCount = 0;
     }
-    
+
     CloudConfig ccfg = cc.getConfig().getCloudConfig();
     Integer seconds = ccfg.getCreateCollectionWaitTimeTillActive();
     Boolean checkLeaderOnly = ccfg.isCreateCollectionCheckLeaderActive();
@@ -1298,7 +1298,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
           // the collection was not created, don't wait
           return true;
         }
-        
+
         if (c.getSlices() != null) {
           Collection<Slice> shards = c.getSlices();
           int replicaNotAliveCnt = 0;
@@ -1326,13 +1326,13 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
         return false;
       });
     } catch (TimeoutException | InterruptedException e) {
-   
+
       String  error = "Timeout waiting for active collection " + collectionName + " with timeout=" + seconds;
       throw new NotInClusterStateException(ErrorCode.SERVER_ERROR, error);
     }
-    
+
   }
-  
+
   public static void verifyRuleParams(CoreContainer cc, Map<String, Object> m) {
     List l = (List) m.get(RULE);
     if (l != null) {
@@ -1368,7 +1368,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
     }
     return props;
   }
-  
+
   private static void verifyShardsParam(String shardsParam) {
     for (String shard : shardsParam.split(",")) {
       SolrIdentifierValidator.validateShardName(shard);


[lucene-solr] 04/09: SOLR-13150: throw exception if not all required params were supplied

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 3541516a7640b307926df1949a54310b97cea403
Author: internet <in...@localhost.localdomain>
AuthorDate: Mon Feb 18 13:59:49 2019 +0200

    SOLR-13150: throw exception if not all required params were supplied
---
 .../apache/solr/cloud/api/collections/CategoryRoutedAlias.java   | 9 ++++++---
 .../java/org/apache/solr/handler/admin/CollectionsHandler.java   | 6 ++++++
 .../update/processor/CategoryRoutedAliasUpdateProcessorTest.java | 2 +-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index 0cd0176..88b68ea 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -48,6 +48,9 @@ public class CategoryRoutedAlias implements RoutedAlias {
   // receive data.
   public static final String UNINITIALIZED = "NEW_CATEGORY_ROUTED_ALIAS_WAITING_FOR_DATA__TEMP";
 
+  @SuppressWarnings("WeakerAccess")
+  public static final String ROUTER_MAX_CARDINALITY = "router.maxCardinality";
+
   /**
    * Parameters required for creating a category routed alias
    */
@@ -55,11 +58,11 @@ public class CategoryRoutedAlias implements RoutedAlias {
   public static final Set<String> REQUIRED_ROUTER_PARAMS = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
       CommonParams.NAME,
       ROUTER_TYPE_NAME,
-      ROUTER_FIELD)));
+      ROUTER_FIELD,
+      ROUTER_MAX_CARDINALITY
+  )));
 
   @SuppressWarnings("WeakerAccess")
-  public static final String ROUTER_MAX_CARDINALITY = "router.maxCardinality";
-  @SuppressWarnings("WeakerAccess")
   public static final String ROUTER_MUST_MATCH = "router.mustMatch";
 
   /**
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 3f39808..293f731 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
@@ -37,6 +37,7 @@ import java.util.stream.Collectors;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.solr.api.Api;
@@ -82,6 +83,7 @@ import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CloudConfig;
 import org.apache.solr.core.CoreContainer;
@@ -582,6 +584,10 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
 
       // Now filter out just the parameters we care about from the request
       Map<String, Object> result = copy(req.getParams(), null, routedAlias.getRequiredParams());
+      if (!result.keySet().containsAll(routedAlias.getRequiredParams())) {
+        throw new SolrException(BAD_REQUEST, "Not all required params were supplied. Missing params: " +
+            StrUtils.join(Sets.difference(routedAlias.getRequiredParams(), result.keySet()), ','));
+      }
       copy(req.getParams(), result, routedAlias.getOptionalParams());
 
       ModifiableSolrParams createCollParams = new ModifiableSolrParams(); // without prefix
diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index d269f8f..ee7fbb5 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -191,7 +191,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
 
     CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
         CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
-            .setMaxShardsPerNode(2))
+            .setMaxShardsPerNode(2)).setMaxCardinality(Integer.MAX_VALUE)
         .process(solrClient);
 
     // now we index a document


[lucene-solr] 08/09: SOLR-13150 precommit fixes

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 326d8c91ea4491ffa5d95c7343010830e72950c0
Author: Gus Heck <gu...@apache.org>
AuthorDate: Tue Feb 19 09:47:25 2019 -0500

    SOLR-13150 precommit fixes
---
 .../core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java | 2 --
 1 file changed, 2 deletions(-)

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 c448c71..751a47c 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
@@ -37,7 +37,6 @@ import java.util.stream.Collectors;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.solr.api.Api;
@@ -83,7 +82,6 @@ import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
-import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CloudConfig;
 import org.apache.solr.core.CoreContainer;


[lucene-solr] 02/09: SOLR-13150: improve assertions for CRA max cardinality test

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit f1ed5750e5c7007bc7008311ad9504a72b5f92f7
Author: internet <in...@localhost.localdomain>
AuthorDate: Sun Feb 17 08:59:58 2019 +0200

    SOLR-13150: improve assertions for CRA max cardinality test
---
 .../update/processor/CategoryRoutedAliasUpdateProcessorTest.java   | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index 3df7784..9cd44fc 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -223,7 +223,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     String configName = getSaferTestName();
     createConfigSet(configName);
 
-    final int maxCardinality = 2;
+    final int maxCardinality = 2; // max cardinality for current test
 
     // Start with one collection manually created (and use higher numShards & replicas than we'll use for others)
     //  This tests we may pre-create the collection and it's acceptable.
@@ -242,7 +242,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
 
     CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
         CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
-            .setMaxShardsPerNode(2)).setMaxCardinality(2)
+            .setMaxShardsPerNode(2)).setMaxCardinality(maxCardinality)
         .process(solrClient);
 
     // now we index a document
@@ -257,7 +257,8 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     assertInvariants(colVogon, colHoG);
 
     // should fail since max cardinality is reached
-    Throwable e = expectThrows(Throwable.class, () -> addDocsAndCommit(true, newDoc(SHIPS[2])));
+    SolrException e = expectThrows(SolrException.class, () ->
+        solrClient.add(getAlias(), Collections.singleton(newDoc(SHIPS[2])), -1));
     assertTrue("update should fail because CRA max cardinality is reached",
         e.getMessage().contains("max cardinality can not be exceeded for a Category Routed Alias"));
     --lastDocId; // since last doc was not indexed


[lucene-solr] 01/09: SOLR-13150: first version

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit e142e851159222be3d38958269c26fb86b6b8b65
Author: internet <in...@localhost.localdomain>
AuthorDate: Thu Feb 14 19:44:54 2019 +0200

    SOLR-13150: first version
---
 .../cloud/api/collections/CategoryRoutedAlias.java | 24 ++++++++++-
 .../CategoryRoutedAliasUpdateProcessorTest.java    | 48 ++++++++++++++++++++++
 .../solrj/request/CollectionAdminRequest.java      | 11 +++++
 3 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index 3de1e41..c337ed5 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -106,6 +106,27 @@ public class CategoryRoutedAlias implements RoutedAlias {
   @Override
   public void validateRouteValue(AddUpdateCommand cmd) throws SolrException {
     //Mostly this will be filled out by SOLR-13150 and SOLR-13151
+    String maxCardinality = aliasMetadata.get(ROUTER_MAX_CARDINALITY);
+    if(maxCardinality == null) {
+      return;
+    }
+
+    if (this.parsedAliases == null) {
+      updateParsedCollectionAliases(cmd.getReq().getCore().getCoreContainer().getZkController());
+    }
+
+    String dataValue = String.valueOf(cmd.getSolrInputDocument().getFieldValue(getRouteField()));
+    String candidateCollectionName = buildCollectionNameFromValue(dataValue);
+    List<String> colls = getCollectionList(this.parsedAliases);
+
+    if (colls.contains(candidateCollectionName)) {
+      return;
+    }
+
+    if (colls.stream()
+        .filter(x -> !x.contains(UNINITIALIZED)).count() >= Integer.valueOf(maxCardinality)) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "max cardinality can not be exceeded for a Category Routed Alias: " + maxCardinality);
+    }
   }
 
   /**
@@ -130,8 +151,7 @@ public class CategoryRoutedAlias implements RoutedAlias {
     CoreContainer coreContainer = core.getCoreContainer();
     CollectionsHandler collectionsHandler = coreContainer.getCollectionsHandler();
     String dataValue = String.valueOf(cmd.getSolrInputDocument().getFieldValue(getRouteField()));
-
-   String candidateCollectionName = buildCollectionNameFromValue(dataValue);
+    String candidateCollectionName = buildCollectionNameFromValue(dataValue);
 
     try {
       // Note: CRA's have no way to predict values that determine collection so preemptive async creation
diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index 566e27d..3df7784 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -31,6 +31,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.ConfigSetAdminRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.cloud.api.collections.CategoryRoutedAlias;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.request.SolrQueryRequest;
@@ -216,6 +217,53 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
   private String noDollar(String ship) {
     return ship.replaceAll("\\$", "_");
   }
+  @Slow
+  @Test
+  public void testMaxCardinality() throws Exception {
+    String configName = getSaferTestName();
+    createConfigSet(configName);
+
+    final int maxCardinality = 2;
+
+    // Start with one collection manually created (and use higher numShards & replicas than we'll use for others)
+    //  This tests we may pre-create the collection and it's acceptable.
+    final String colVogon = getAlias() + "__CRA__" + SHIPS[0];
+
+    // we expect changes ensuring a legal collection name.
+    final String colHoG = getAlias() + "__CRA__" + SHIPS[1].replaceAll("\\s", "_");
+
+    List<String> retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets();
+    List<String> expectedConfigSetNames = Arrays.asList("_default", configName);
+
+    // config sets leak between tests so we can't be any more specific than this on the next 2 asserts
+    assertTrue("We expect at least 2 configSets",
+        retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
+    assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
+
+    CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+        CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
+            .setMaxShardsPerNode(2)).setMaxCardinality(2)
+        .process(solrClient);
+
+    // now we index a document
+    addDocsAndCommit(true, newDoc(SHIPS[0]));
+    //assertDocRoutedToCol(lastDocId, col23rd);
+
+    String uninitialized = getAlias() + "__CRA__" + CategoryRoutedAlias.UNINITIALIZED;
+    assertInvariants(colVogon, uninitialized);
+
+    addDocsAndCommit(true, newDoc(SHIPS[1]));
+
+    assertInvariants(colVogon, colHoG);
+
+    // should fail since max cardinality is reached
+    Throwable e = expectThrows(Throwable.class, () -> addDocsAndCommit(true, newDoc(SHIPS[2])));
+    assertTrue("update should fail because CRA max cardinality is reached",
+        e.getMessage().contains("max cardinality can not be exceeded for a Category Routed Alias"));
+    --lastDocId; // since last doc was not indexed
+    assertInvariants(colVogon, colHoG);
+  }
+
 
   /**
    * Test that the Update Processor Factory routes documents to leader shards and thus
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 d0fc34b..eac1006 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
@@ -1632,9 +1632,11 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
 
     public static final String ROUTER_TYPE_NAME = "router.name";
     public static final String ROUTER_FIELD = "router.field";
+    public static final String ROUTER_MAX_CARDINALITY = "router.maxCardinality";
 
     private final String aliasName;
     private final String routerField;
+    private Integer maxCardinality;
 
     private final Create createCollTemplate;
 
@@ -1645,6 +1647,11 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
       this.createCollTemplate = createCollTemplate;
     }
 
+    public CreateCategoryRoutedAlias setMaxCardinality(int maxCardinality) {
+      this.maxCardinality = maxCardinality;
+      return this;
+    }
+
     @Override
     public SolrParams getParams() {
       ModifiableSolrParams params = (ModifiableSolrParams) super.getParams();
@@ -1652,6 +1659,10 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
       params.add(ROUTER_TYPE_NAME, "category");
       params.add(ROUTER_FIELD, routerField);
 
+      if (maxCardinality != null) {
+        params.add(ROUTER_MAX_CARDINALITY, maxCardinality.toString());
+      }
+
       // merge the above with collectionParams.  Above takes precedence.
       ModifiableSolrParams createCollParams = new ModifiableSolrParams(); // output target
       final SolrParams collParams = createCollTemplate.getParams();


[lucene-solr] 03/09: SOLR-13150: fix CRA max cardinality test compatiblity with TolerantUpdateProcessor

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 1208d694d82610cfd4c30ad306f4e8d789578133
Author: internet <in...@localhost.localdomain>
AuthorDate: Sun Feb 17 11:46:21 2019 +0200

    SOLR-13150: fix CRA max cardinality test compatiblity with TolerantUpdateProcessor
---
 .../cloud/api/collections/CategoryRoutedAlias.java  |  8 ++++----
 .../CategoryRoutedAliasUpdateProcessorTest.java     | 21 +++++++++++++++------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index c337ed5..0cd0176 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -117,15 +117,15 @@ public class CategoryRoutedAlias implements RoutedAlias {
 
     String dataValue = String.valueOf(cmd.getSolrInputDocument().getFieldValue(getRouteField()));
     String candidateCollectionName = buildCollectionNameFromValue(dataValue);
-    List<String> colls = getCollectionList(this.parsedAliases);
+    List<String> cols = getCollectionList(this.parsedAliases);
 
-    if (colls.contains(candidateCollectionName)) {
+    if (cols.contains(candidateCollectionName)) {
       return;
     }
 
-    if (colls.stream()
+    if (cols.stream()
         .filter(x -> !x.contains(UNINITIALIZED)).count() >= Integer.valueOf(maxCardinality)) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "max cardinality can not be exceeded for a Category Routed Alias: " + maxCardinality);
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "max cardinality can not be exceeded for a Category Routed Alias: " + maxCardinality);
     }
   }
 
diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index 9cd44fc..d269f8f 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -30,6 +30,7 @@ import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.ConfigSetAdminRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.cloud.api.collections.CategoryRoutedAlias;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
@@ -62,6 +63,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
 
   private int lastDocId = 0;
   private static CloudSolrClient solrClient;
+  private int numDocsDeletedOrFailed = 0;
 
   @Before
   public void doBefore() throws Exception {
@@ -257,11 +259,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     assertInvariants(colVogon, colHoG);
 
     // should fail since max cardinality is reached
-    SolrException e = expectThrows(SolrException.class, () ->
-        solrClient.add(getAlias(), Collections.singleton(newDoc(SHIPS[2])), -1));
-    assertTrue("update should fail because CRA max cardinality is reached",
-        e.getMessage().contains("max cardinality can not be exceeded for a Category Routed Alias"));
-    --lastDocId; // since last doc was not indexed
+    testFailedDocument(newDoc(SHIPS[2]), "max cardinality can not be exceeded for a Category Routed Alias");
     assertInvariants(colVogon, colHoG);
   }
 
@@ -326,7 +324,6 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
 
 
   private void assertInvariants(String... expectedColls) throws IOException, SolrServerException {
-    int numDocsDeletedOrFailed = 0;
     final int expectNumFound = lastDocId - numDocsDeletedOrFailed; //lastDocId is effectively # generated docs
 
     List<String> cols = new CollectionAdminRequest.ListAliases().process(solrClient).getAliasesAsLists().get(getAlias());
@@ -384,4 +381,16 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     }
   }
 
+  private void testFailedDocument(SolrInputDocument sdoc, String errorMsg) throws SolrServerException, IOException {
+    try {
+      final UpdateResponse resp = solrClient.add(getAlias(), sdoc);
+      // if we have a TolerantUpdateProcessor then we see it there)
+      final Object errors = resp.getResponseHeader().get("errors"); // Tolerant URP
+      assertTrue(errors != null && errors.toString().contains(errorMsg));
+    } catch (SolrException e) {
+      assertTrue(e.getMessage().contains(errorMsg));
+    }
+    ++numDocsDeletedOrFailed;
+  }
+
 }


[lucene-solr] 06/09: SOLR-13150 fix tests moshe didn't have access to and remove my print statements from previous commit.

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 31cacfdd0eb23a2b6109a0cef5e31f122c227c51
Author: Gus Heck <gu...@apache.org>
AuthorDate: Mon Feb 18 22:50:49 2019 -0500

    SOLR-13150 fix tests moshe didn't have access to and remove my print statements from previous commit.
---
 .../processor/CategoryRoutedAliasUpdateProcessorTest.java    | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index 8607a52..f139828 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -121,19 +121,15 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     List<String> retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets();
     List<String> expectedConfigSetNames = Arrays.asList("_default", configName);
 
-    System.out.println("*****************FOO***************");
-
     // config sets leak between tests so we can't be any more specific than this on the next 2 asserts
     assertTrue("We expect at least 2 configSets",
         retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
     assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
 
-    System.out.println("*****************FOO2***************");
     CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
         CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
-            .setMaxShardsPerNode(2))
+            .setMaxShardsPerNode(2)).setMaxCardinality(20)
         .process(solrClient);
-    System.out.println("*****************FOO3***************");
     addDocsAndCommit(true,
         newDoc(somethingInChinese),
         newDoc(somethingInHebrew),
@@ -142,13 +138,9 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
         newDoc(somethingInGreek),
         newDoc(somethingInGujarati));
 
-    System.out.println("*****************FOO4***************");
-
     // Note Gujarati not listed, because it duplicates hebrew.
     assertInvariants(collectionChinese, collectionHebrew, collectionThai, collectionArabic, collectionGreek);
 
-    System.out.println("*****************FOO5***************");
-
     assertColHasDocCount(collectionChinese, 1);
     assertColHasDocCount(collectionHebrew, 2);
     assertColHasDocCount(collectionThai, 1);
@@ -305,7 +297,7 @@ public class CategoryRoutedAliasUpdateProcessorTest extends RoutedAliasUpdatePro
     final int numReplicas = 1 + random().nextInt(3);
     CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
         CollectionAdminRequest.createCollection("_unused_", configName, numShards, numReplicas)
-            .setMaxShardsPerNode(numReplicas))
+            .setMaxShardsPerNode(numReplicas)).setMaxCardinality(20)
         .process(solrClient);
 
     // cause some collections to be created


[lucene-solr] 09/09: SOLR-13150 add some javadoc

Posted by gu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gus pushed a commit to branch solr-13131
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 01737e16c62e23e8034adda7a0b7ffaa61cf3388
Author: Gus Heck <gu...@apache.org>
AuthorDate: Tue Feb 19 09:51:50 2019 -0500

    SOLR-13150 add some javadoc
---
 .../apache/solr/cloud/api/collections/CategoryRoutedAlias.java    | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index a5a5bce..bf32609 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -148,6 +148,14 @@ public class CategoryRoutedAlias implements RoutedAlias {
     return aliasName + COLLECTION_INFIX + safeKeyValue(value);
   }
 
+  /**
+   * Method to possibly create a collection. It's possible that the collection will already have been created
+   * either by a prior invocation in this thread or another thread. This method is idempotent, multiple invocations
+   * are harmless.
+   *
+   * @param cmd The command that might cause collection creation
+   * @return the collection to which the the update should be directed, possibly a newly created collection.
+   */
   @Override
   public String createCollectionsIfRequired(AddUpdateCommand cmd) {
     SolrQueryRequest req = cmd.getReq();