lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a.@apache.org
Subject [lucene-solr] branch branch_8x updated: SOLR-13475: Null Pointer Exception when querying collection through collection alias.
Date Mon, 20 May 2019 11:35:17 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 97d0d46  SOLR-13475: Null Pointer Exception when querying collection through collection
alias.
97d0d46 is described below

commit 97d0d46db646a39c5dba24b110416e773c92bf2f
Author: Andrzej Bialecki <ab@apache.org>
AuthorDate: Mon May 20 13:35:00 2019 +0200

    SOLR-13475: Null Pointer Exception when querying collection through collection alias.
---
 solr/CHANGES.txt                                   |  5 +++
 .../solr/cloud/api/collections/CreateAliasCmd.java |  4 ++
 .../cloud/api/collections/CreateCollectionCmd.java |  6 ++-
 .../cloud/api/collections/DeleteCollectionCmd.java | 48 ++++++++++++++--------
 .../apache/solr/cloud/AliasIntegrationTest.java    | 12 +++++-
 solr/solr-ref-guide/src/collections-api.adoc       |  4 +-
 .../java/org/apache/solr/common/cloud/Aliases.java |  8 +++-
 7 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 2c5f4ee..e8c6db5 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -50,6 +50,11 @@ New Features
 * SOLR-12304: The MoreLikeThisComponent now supports the mlt.interestingTerms parameter.
 Previously this option was
   unique to the MLT handler.   (Alessandro Benedetti via David Smiley)
 
+Bug Fixes
+----------------------
+
+* SOLR-13475: Null Pointer Exception when querying collection through collection alias. (Jörn
Franke, ab)
+
 Other Changes
 ----------------------
 
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 57be84f..8d0d45c 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
@@ -85,6 +85,9 @@ public class CreateAliasCmd extends AliasCmd {
 
   private void callCreatePlainAlias(ZkNodeProps message, String aliasName, ZkStateReader
zkStateReader) {
     final List<String> canonicalCollectionList = parseCollectionsParameter(message.get("collections"));
+    if (canonicalCollectionList.isEmpty()) {
+      throw new SolrException(BAD_REQUEST, "'collections' parameter doesn't contain any collection
names.");
+    }
     final String canonicalCollectionsString = StrUtils.join(canonicalCollectionList, ',');
     validateAllCollectionsExistAndNoDuplicates(canonicalCollectionList, zkStateReader);
     zkStateReader.aliasesManager
@@ -101,6 +104,7 @@ public class CreateAliasCmd extends AliasCmd {
     if (colls instanceof List) return (List<String>) colls;
     return StrUtils.splitSmart(colls.toString(), ",", true).stream()
         .map(String::trim)
+        .filter(s -> !s.isEmpty())
         .collect(Collectors.toList());
   }
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index a71cb53..dfd5a21 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -329,8 +329,10 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
         }
       }
 
-      // create an alias pointing to the new collection
-      ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias(alias,
collectionName));
+      // create an alias pointing to the new collection, if different from the collectionName
+      if (!alias.equals(collectionName)) {
+        ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias(alias,
collectionName));
+      }
 
     } catch (SolrException ex) {
       throw ex;
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
index d5215f7..2054258 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java
@@ -25,6 +25,7 @@ import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
 import static org.apache.solr.common.params.CommonParams.NAME;
 
 import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -75,7 +76,7 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
       zkStateReader.aliasesManager.update(); // aliases may have been stale; get latest from
ZK
     }
 
-    String aliasReference = checkAliasReference(zkStateReader, extCollection);
+    List<String> aliasReferences = checkAliasReference(zkStateReader, extCollection);
 
     Aliases aliases = zkStateReader.getAliases();
     String collection = aliases.resolveSimpleAlias(extCollection);
@@ -135,9 +136,14 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
       // wait for a while until we don't see the collection
       zkStateReader.waitForState(collection, 60, TimeUnit.SECONDS, (liveNodes, collectionState)
-> collectionState == null);
 
-      // we can delete any remaining unique alias
-      if (aliasReference != null) {
-        ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias(aliasReference,
null));
+      // we can delete any remaining unique aliases
+      if (!aliasReferences.isEmpty()) {
+        ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> {
+          for (String alias : aliasReferences) {
+            a = a.cloneWithCollectionAlias(alias, null);
+          }
+          return a;
+        });
       }
 
 //      TimeOut timeout = new TimeOut(60, TimeUnit.SECONDS, timeSource);
@@ -178,23 +184,29 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
     }
   }
 
-  // it's ok if a collection is referenced either by none or exactly by a single alias.
-  // This method returns the single alias to delete, if present, or null
-  private String checkAliasReference(ZkStateReader zkStateReader, String extCollection) throws
Exception {
-    List<String> aliases = referencedByAlias(extCollection, zkStateReader.getAliases());
-    if (aliases.size() > 1) {
+  // This method returns the single collection aliases to delete, if present, or null
+  private List<String> checkAliasReference(ZkStateReader zkStateReader, String extCollection)
throws Exception {
+    Aliases aliases = zkStateReader.getAliases();
+    List<String> aliasesRefs = referencedByAlias(extCollection, aliases);
+    List<String> aliasesToDelete = new ArrayList<>();
+    if (aliasesRefs.size() > 0) {
       zkStateReader.aliasesManager.update(); // aliases may have been stale; get latest from
ZK
-      aliases = referencedByAlias(extCollection, zkStateReader.getAliases());
-      if (aliases.size() > 1) {
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-            "Collection : " + extCollection + " is part of aliases: " + aliases + ", remove
or modify the aliases before removing this collection.");
+      aliases = zkStateReader.getAliases();
+      aliasesRefs = referencedByAlias(extCollection, aliases);
+      if (aliasesRefs.size() > 0) {
+        for (String alias : aliasesRefs) {
+          // for back-compat in 8.x we don't automatically remove other
+          // aliases that point only to this collection
+          if (!extCollection.equals(alias)) {
+            throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                "Collection : " + extCollection + " is part of aliases: " + aliasesRefs +
", remove or modify the aliases before removing this collection.");
+          } else {
+            aliasesToDelete.add(alias);
+          }
+        }
       }
     }
-    if (!aliases.isEmpty()) {
-      return aliases.get(0);
-    } else {
-      return null;
-    }
+    return aliasesToDelete;
   }
 
   public static List<String> referencedByAlias(String extCollection, Aliases aliases)
throws IllegalArgumentException {
diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
index 2ee0cfd..4a636e2 100644
--- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java
@@ -544,8 +544,18 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
 
     ///////////////
     CollectionAdminRequest.createAlias("testalias1", "collection1").process(cluster.getSolrClient());
+    // make sure there's only one level of alias
+    CollectionAdminRequest.deleteAlias("collection1").process(cluster.getSolrClient());
 
-    // ensure that the alias has been registered
+    // verify proper resolution on the server-side
+    ZkStateReader zkStateReader = cluster.getSolrClient().getZkStateReader();
+    zkStateReader.aliasesManager.update();
+    Aliases aliases = zkStateReader.getAliases();
+    List<String> collections = aliases.resolveAliases("testalias1");
+    assertEquals(collections.toString(), 1, collections.size());
+    assertTrue(collections.contains("collection1"));
+
+    // ensure that the alias is visible in the API
     assertEquals("collection1",
         new CollectionAdminRequest.ListAliases().process(cluster.getSolrClient()).getAliases().get("testalias1"));
 
diff --git a/solr/solr-ref-guide/src/collections-api.adoc b/solr/solr-ref-guide/src/collections-api.adoc
index 1c39a1b..af7f63b 100644
--- a/solr/solr-ref-guide/src/collections-api.adoc
+++ b/solr/solr-ref-guide/src/collections-api.adoc
@@ -121,8 +121,8 @@ The name of the collection with which all replicas of this collection
must be co
 See <<colocating-collections.adoc#colocating-collections, Colocating collections>>
for more details.
 
 `alias`::
-Starting with version 8.1 when a collection is created additionally an alias (by default
with the same name) is created
-that points to this collection. This parameter allows changing the name of this alias, effectively
combining
+Starting with version 8.1 when a collection is created additionally an alias can be created
+that points to this collection. This parameter allows specifying the name of this alias,
effectively combining
 this operation with <<createalias, CREATEALIAS>>
 
 Collections are first created in read-write mode but can be put in `readOnly`
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
index 6f97c25..c19ee58 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
@@ -248,11 +248,15 @@ public class Aliases {
     for (int i = 0; i < level1.size(); i++) {
       String level1Alias = level1.get(i);
       List<String> level2 = collectionAliasListMap.get(level1Alias);
-      if (level2 == null && uniqueResult != null) {
-        uniqueResult.add(level1Alias);
+      if (level2 == null) {
+        // will copy all level1alias-es so far on lazy init
+        if (uniqueResult != null) {
+          uniqueResult.add(level1Alias);
+        }
       } else {
         if (uniqueResult == null) { // lazy init
           uniqueResult = new LinkedHashSet<>(level1.size());
+          // add all level1Alias-es so far
           uniqueResult.addAll(level1.subList(0, i));
         }
         uniqueResult.addAll(level2);


Mime
View raw message