You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ge...@apache.org on 2024/01/19 18:18:32 UTC

(solr) branch branch_9x updated: SOLR-17116: Fix INSTALLSHARDDATA async reporting (#2188)

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

gerlowskija pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9x by this push:
     new cccae345c2f SOLR-17116: Fix INSTALLSHARDDATA async reporting (#2188)
cccae345c2f is described below

commit cccae345c2fd599c8b8bfc68b02f25a4f593723a
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Fri Jan 19 12:59:18 2024 -0500

    SOLR-17116: Fix INSTALLSHARDDATA async reporting (#2188)
    
    Prior to this commit, failures in asynchronous 'INSTALLSHARDDATA'
    commands weren't properly detected.  Instead the task was always labeled
    as 'completed', even when it failed.
    
    Ultimately, this occurred because the overseer processing for this
    command ignored the output of per-core requests.  Incorporating these
    responses into the overseer "results" allows task operation to be
    correctly classified and reported.
---
 solr/CHANGES.txt                                   |  2 +
 .../cloud/api/collections/InstallShardDataCmd.java |  2 +-
 .../api/collections/AbstractInstallShardTest.java  | 45 +++++++++++++++++++++-
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 274e8f99179..b1a12d5680f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -94,6 +94,8 @@ Bug Fixes
 
 * SOLR-17121: Fix SchemaCodecFactory to get PostingsFormat and DocValues from field. (Bruno Roustant, David Smiley)
 
+* SOLR-17116: The INSTALLSHARDDATA "collection-admin" API now reports errors correctly when run asynchronously. (Jason Gerlowski)
+
 Dependency Upgrades
 ---------------------
 * SOLR-17012: Update Apache Hadoop to 3.3.6 and Apache Curator to 5.5.0 (Kevin Risden)
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/InstallShardDataCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/InstallShardDataCmd.java
index f3c9cb1dec2..61549a64194 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/InstallShardDataCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/InstallShardDataCmd.java
@@ -91,7 +91,7 @@ public class InstallShardDataCmd implements CollApiCmds.CollectionApiCommand {
             "Could not install data to collection [%s] and shard [%s]",
             typedMessage.collection,
             typedMessage.shard);
-    shardRequestTracker.processResponses(new NamedList<>(), shardHandler, true, errorMessage);
+    shardRequestTracker.processResponses(results, shardHandler, true, errorMessage);
   }
 
   /** A value-type representing the message received by {@link InstallShardDataCmd} */
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
index 497e8e62cf8..2b5ae0f7c51 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractInstallShardTest.java
@@ -39,6 +39,7 @@ import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.response.RequestStatusState;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.util.ExecutorUtil;
@@ -101,6 +102,7 @@ public abstract class AbstractInstallShardTest extends SolrCloudTestCase {
   private static int replicasPerShard = -1;
   private static int multiShardNumDocs = -1;
   private static URI singleShard1Uri = null;
+  private static URI nonExistentLocationUri = null;
   private static URI[] multiShardUris = null;
 
   private List<String> collectionsToDelete;
@@ -124,6 +126,9 @@ public abstract class AbstractInstallShardTest extends SolrCloudTestCase {
         createBackupRepoDirectoryForShardData(
             baseRepositoryLocation, singleShardCollName, "shard1");
     copyShardDataToBackupRepository(singleShardCollName, "shard1", singleShard1Uri);
+    // Based off the single-shard location and used for error-case testing
+    nonExistentLocationUri = createUriForNonexistentSubDir(singleShard1Uri, "nonexistent");
+
     // Upload shard data to BackupRepository - multi-shard collection
     for (int i = 0; i < multiShardUris.length; i++) {
       final String shardName = "shard" + (i + 1);
@@ -170,10 +175,40 @@ public abstract class AbstractInstallShardTest extends SolrCloudTestCase {
             collectionName, "shard1", singleShardLocation, BACKUP_REPO_NAME)
         .process(cluster.getSolrClient());
 
-    // Shard-install has failed so collection should still be empty.
     assertCollectionHasNumDocs(collectionName, singleShardNumDocs);
   }
 
+  @Test
+  public void testInstallReportsErrorsAppropriately() throws Exception {
+    final String collectionName = createAndAwaitEmptyCollection(1, replicasPerShard);
+    deleteAfterTest(collectionName);
+    enableReadOnly(collectionName);
+    final String nonExistentLocation = nonExistentLocationUri.toString();
+
+    { // Test synchronous request error reporting
+      final var expectedException =
+          expectThrows(
+              BaseHttpSolrClient.RemoteSolrException.class,
+              () -> {
+                CollectionAdminRequest.installDataToShard(
+                        collectionName, "shard1", nonExistentLocation, BACKUP_REPO_NAME)
+                    .process(cluster.getSolrClient());
+              });
+      assertTrue(expectedException.getMessage().contains("Could not install data to collection"));
+      assertCollectionHasNumDocs(collectionName, 0);
+    }
+
+    { // Test asynchronous request error reporting
+      final var requestStatusState =
+          CollectionAdminRequest.installDataToShard(
+                  collectionName, "shard1", nonExistentLocation, BACKUP_REPO_NAME)
+              .processAndWait(cluster.getSolrClient(), 15);
+
+      assertEquals(RequestStatusState.FAILED, requestStatusState);
+      assertCollectionHasNumDocs(collectionName, 0);
+    }
+  }
+
   @Test
   public void testSerialInstallToMultiShardCollection() throws Exception {
     final String collectionName =
@@ -296,6 +331,14 @@ public abstract class AbstractInstallShardTest extends SolrCloudTestCase {
     }
   }
 
+  private static URI createUriForNonexistentSubDir(URI baseUri, String subDirName)
+      throws Exception {
+    final CoreContainer cc = cluster.getJettySolrRunner(0).getCoreContainer();
+    try (final BackupRepository backupRepository = cc.newBackupRepository(BACKUP_REPO_NAME)) {
+      return backupRepository.resolve(baseUri, subDirName);
+    }
+  }
+
   private static int indexDocs(String collectionName, boolean useUUID) throws Exception {
     Random random =
         new Random(