SOLR-12308: LISTALIASES is now assured to return an up-to-date response

* MiniSolrCloudCluster.deleteAllCollections will now first delete aliases
* Minor refactorings to AliasesManager, AliasIntegrationTest, CreateRoutedAliasTest

solr/CHANGES.txt

@@ -215,6 +215,10 @@ Bug Fixes
 * SOLR-12316: Do not allow to use absolute URIs for including other files in solrconfig.xml and schema parsing.
   (Ananthesh, Ishan Chattopadhyaya, Uwe Schindler)
 
+* SOLR-12308: LISTALIASES is now assured to return an up-to-date response.  Also, the test utility
+  MiniSolrCloudCluster.deleteAllCollections will now first delete aliases since a collection cannot be deleted if an
+  alias refers to it.  (David Smiley)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java

@@ -590,6 +590,8 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
      */
     LISTALIASES_OP(LISTALIASES, (req, rsp, h) -> {
       ZkStateReader zkStateReader = h.coreContainer.getZkController().getZkStateReader();
+      // if someone calls listAliases, lets ensure we return an up to date response
+      zkStateReader.aliasesManager.update();
       Aliases aliases = zkStateReader.getAliases();
       if (aliases != null) {
         // the aliases themselves...

solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java

@@ -83,13 +83,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
     super.tearDown();
     IOUtils.close(solrClient, httpClient);
 
-    // make sure all aliases created are removed for the next test method
-    Map<String, String> aliases = new CollectionAdminRequest.ListAliases().process(cluster.getSolrClient()).getAliases();
-    for (String alias : aliases.keySet()) {
-      CollectionAdminRequest.deleteAlias(alias).process(cluster.getSolrClient());
-    }
-
-    cluster.deleteAllCollections();
+    cluster.deleteAllCollections(); // note: deletes aliases too
   }
 
   @Test
@@ -104,10 +98,9 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
     List<String> aliases = zkStateReader.getAliases().resolveAliases("meta1");
     assertEquals(1, aliases.size());
     assertEquals("meta1", aliases.get(0));
-    UnaryOperator<Aliases> op6 = a -> a.cloneWithCollectionAlias("meta1", "collection1meta,collection2meta");
     final ZkStateReader.AliasesManager aliasesManager = zkStateReader.aliasesManager;
 
-    aliasesManager.applyModificationAndExportToZk(op6);
+    aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias("meta1", "collection1meta,collection2meta"));
     aliases = zkStateReader.getAliases().resolveAliases("meta1");
     assertEquals(2, aliases.size());
     assertEquals("collection1meta", aliases.get(0));
@@ -186,35 +179,28 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
 
     // now check that an independently constructed ZkStateReader can see what we've done.
     // i.e. the data is really in zookeeper
-    String zkAddress = cluster.getZkServer().getZkAddress();
-    boolean createdZKSR = false;
-    try(SolrZkClient zkClient = new SolrZkClient(zkAddress, 30000)) {
-
+    try (SolrZkClient zkClient = new SolrZkClient(cluster.getZkServer().getZkAddress(), 30000)) {
       ZkController.createClusterZkNodes(zkClient);
+      try (ZkStateReader zkStateReader2 = new ZkStateReader(zkClient)) {
+        zkStateReader2.createClusterStateWatchersAndUpdate();
 
-      zkStateReader = new ZkStateReader(zkClient);
-      createdZKSR = true;
-      zkStateReader.createClusterStateWatchersAndUpdate();
+        meta = zkStateReader2.getAliases().getCollectionAliasProperties("meta1");
+        assertNotNull(meta);
 
-      meta = zkStateReader.getAliases().getCollectionAliasProperties("meta1");
-      assertNotNull(meta);
+        // verify key was removed in independent view
+        assertFalse(meta.containsKey("foo"));
 
-      // verify key was removed in independent view
-      assertFalse(meta.containsKey("foo"));
-
-      // but only the specified key was removed
-      assertTrue(meta.containsKey("foobar"));
-      assertEquals("bazbam", meta.get("foobar"));
-
-      Aliases a = zkStateReader.getAliases();
-      Aliases clone = a.cloneWithCollectionAlias("meta1", null);
-      meta = clone.getCollectionAliasProperties("meta1");
-      assertEquals(0,meta.size());
-    } finally {
-      if (createdZKSR) {
-        zkStateReader.close();
+        // but only the specified key was removed
+        assertTrue(meta.containsKey("foobar"));
+        assertEquals("bazbam", meta.get("foobar"));
       }
     }
+
+    // check removal leaves no props behind
+    assertEquals(0, zkStateReader.getAliases()
+        .cloneWithCollectionAlias("meta1", null) // not persisted to zk on purpose
+        .getCollectionAliasProperties("meta1")
+        .size());
   }
 
   @Test
@@ -497,7 +483,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase {
 
     ///////////////
     CollectionAdminRequest.createAlias("testalias1", "collection1").process(cluster.getSolrClient());
-    sleepToAllowZkPropagation();
+
     // ensure that the alias has been registered
     assertEquals("collection1",
         new CollectionAdminRequest.ListAliases().process(cluster.getSolrClient()).getAliases().get("testalias1"));

solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java

@@ -32,7 +32,6 @@ import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.util.EntityUtils;
-import org.apache.lucene.util.IOUtils;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrServerException;
@@ -57,9 +56,6 @@ import org.junit.Test;
 @SolrTestCaseJ4.SuppressSSL
 public class CreateRoutedAliasTest extends SolrCloudTestCase {
 
-  private CloudSolrClient solrClient;
-  private CloseableHttpClient httpClient;
-
   @BeforeClass
   public static void setupCluster() throws Exception {
     configureCluster(2).configure();
@@ -73,27 +69,18 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
 //        .process(cluster.getSolrClient());
   }
 
-  @After
-  public void finish() throws Exception {
-    IOUtils.close(solrClient, httpClient);
-  }
+  private CloudSolrClient solrClient;
 
   @Before
   public void doBefore() throws Exception {
     solrClient = getCloudSolrClient(cluster);
-    httpClient = (CloseableHttpClient) solrClient.getHttpClient();
-    // delete aliases first since they refer to the collections
-    ZkStateReader zkStateReader = cluster.getSolrClient().getZkStateReader();
-    //TODO create an API to delete collections attached to the routed alias when the alias is removed
-    zkStateReader.aliasesManager.update();// ensure we're seeing the latest
-    zkStateReader.aliasesManager.applyModificationAndExportToZk(aliases -> {
-      Aliases a = zkStateReader.getAliases();
-      for (String alias : a.getCollectionAliasMap().keySet()) {
-        a = a.cloneWithCollectionAlias(alias,null); // remove
-      }
-      return a;
-    });
-    cluster.deleteAllCollections();
+  }
+
+  @After
+  public void doAfter() throws Exception {
+    cluster.deleteAllCollections(); // deletes aliases too
+
+    solrClient.close();
   }
 
   // This is a fairly complete test where we set many options and see that it both affected the created
@@ -235,7 +222,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
           "+30MINUTE",
           "evt_dt",
           CollectionAdminRequest.createCollection("_ignored_", "_default", 1, 1)
-          )
+      )
           .setTimeZone(TimeZone.getTimeZone("GMT-10"))
           .process(client);
     }
@@ -366,6 +353,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
   }
 
   private void assertSuccess(HttpUriRequest msg) throws IOException {
+    CloseableHttpClient httpClient = (CloseableHttpClient) solrClient.getHttpClient();
     try (CloseableHttpResponse response = httpClient.execute(msg)) {
       if (200 != response.getStatusLine().getStatusCode()) {
         System.err.println(EntityUtils.toString(response.getEntity()));
@@ -375,6 +363,7 @@ public class CreateRoutedAliasTest extends SolrCloudTestCase {
   }
 
   private void assertFailure(HttpUriRequest msg, String expectedErrorSubstring) throws IOException {
+    CloseableHttpClient httpClient = (CloseableHttpClient) solrClient.getHttpClient();
     try (CloseableHttpResponse response = httpClient.execute(msg)) {
       assertEquals(400, response.getStatusLine().getStatusCode());
       String entity = EntityUtils.toString(response.getEntity());

solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java

@@ -1685,9 +1685,7 @@ public class ZkStateReader implements Closeable {
     public void applyModificationAndExportToZk(UnaryOperator<Aliases> op) {
       final long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(30);
       // note: triesLeft tuning is based on ConcurrentCreateRoutedAliasTest
-      int triesLeft = 30;
-      while (triesLeft > 0) {
-        triesLeft--;
+      for (int triesLeft = 30; triesLeft > 0; triesLeft--) {
         // we could synchronize on "this" but there doesn't seem to be a point; we have a retry loop.
         Aliases curAliases = getAliases();
         Aliases modAliases = op.apply(curAliases);
@@ -1723,9 +1721,7 @@ public class ZkStateReader implements Closeable {
           throw new ZooKeeperException(ErrorCode.SERVER_ERROR, e.toString(), e);
         }
       }
-      if (triesLeft == 0) {
-        throw new SolrException(ErrorCode.SERVER_ERROR, "Too many successive version failures trying to update aliases");
-      }
+      throw new SolrException(ErrorCode.SERVER_ERROR, "Too many successive version failures trying to update aliases");
     }
 
     /**
@@ -1734,7 +1730,7 @@ public class ZkStateReader implements Closeable {
      * @return true if an update was performed
      */
     public boolean update() throws KeeperException, InterruptedException {
-      LOG.debug("Checking ZK for most up to date Aliases " + ALIASES);
+      LOG.debug("Checking ZK for most up to date Aliases {}", ALIASES);
       // Call sync() first to ensure the subsequent read (getData) is up to date.
       zkClient.getSolrZooKeeper().sync(ALIASES, null, null);
       Stat stat = new Stat();

solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java

@@ -44,6 +44,7 @@ import org.apache.solr.client.solrj.embedded.SSLConfig;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.CloudSolrClient.Builder;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.common.cloud.Aliases;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkConfigManager;
@@ -444,9 +445,11 @@ public class MiniSolrCloudCluster {
     }
   }
 
+  /** Delete all collections (and aliases) */
   public void deleteAllCollections() throws Exception {
     try (ZkStateReader reader = new ZkStateReader(solrClient.getZkStateReader().getZkClient())) {
-      reader.createClusterStateWatchersAndUpdate();
+      reader.createClusterStateWatchersAndUpdate(); // up to date aliases & collections
+      reader.aliasesManager.applyModificationAndExportToZk(aliases -> Aliases.EMPTY);
       for (String collection : reader.getClusterState().getCollectionStates().keySet()) {
         CollectionAdminRequest.deleteCollection(collection).process(solrClient);
       }