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 17:59:24 UTC

(solr) branch main 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 main
in repository https://gitbox.apache.org/repos/asf/solr.git


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

commit 2a85f47c2093855f6cfe6410ccfc343889e6b7a3
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 0668e944080..b87880ecaa9 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -165,6 +165,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 991577f4e46..18ce5aa4071 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 cc9d655f04c..1cd954966ab 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 =
@@ -295,6 +330,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(