You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2021/02/19 20:51:27 UTC

[lucene-solr] branch branch_8x updated: SOLR-15087: Allow restoration to existing collections (#2380)

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

gerlowskija 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 645e68e  SOLR-15087: Allow restoration to existing collections (#2380)
645e68e is described below

commit 645e68ea061317d910e3de61537d219a4d29a62c
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Tue Feb 16 21:59:24 2021 -0500

    SOLR-15087: Allow restoration to existing collections (#2380)
    
    The recent addition of support for a "readonly" mode for collections
    opens the door to restoring to already-existing collections.
    
    This commit adds a codepath to allow this.  Any compatible existing
    collection may be used for restoration, including the collection that
    was the original source of the backup.
---
 .../OverseerCollectionMessageHandler.java          |  2 +-
 .../solr/cloud/api/collections/RestoreCmd.java     | 58 +++++++++++++-
 .../solr/handler/admin/CollectionsHandler.java     |  5 --
 .../apache/solr/handler/admin/RestoreCoreOp.java   |  4 +-
 solr/solr-ref-guide/src/collection-management.adoc |  6 +-
 .../collections/AbstractIncrementalBackupTest.java | 91 +++++++++++++++++-----
 6 files changed, 134 insertions(+), 32 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
index 9c9963e..2ba3b2c 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
@@ -626,7 +626,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler,
   }
 
 
-  private void modifyCollection(ClusterState clusterState, ZkNodeProps message, @SuppressWarnings({"rawtypes"})NamedList results)
+  void modifyCollection(ClusterState clusterState, ZkNodeProps message, @SuppressWarnings({"rawtypes"})NamedList results)
       throws Exception {
 
     final String collectionName = message.getStr(ZkStateReader.COLLECTION_PROP);
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
index e45119e..493669b 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java
@@ -52,6 +52,7 @@ import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CollectionAdminParams;
+import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -96,8 +97,8 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd {
   public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception {
     try (RestoreContext restoreContext = new RestoreContext(message, ocmh)) {
       if (state.hasCollection(restoreContext.restoreCollectionName)) {
-        throw new SolrException(ErrorCode.BAD_REQUEST, "Restoration collection [" + restoreContext.restoreCollectionName +
-                "] must be created by the backup process and cannot exist");
+        RestoreOnExistingCollection restoreOnExistingCollection = new RestoreOnExistingCollection(restoreContext);
+        restoreOnExistingCollection.process(restoreContext, results);
       } else {
         RestoreOnANewCollection restoreOnANewCollection = new RestoreOnANewCollection(message, restoreContext.backupCollectionState);
         restoreOnANewCollection.validate(restoreContext.backupCollectionState, restoreContext.nodeList.size());
@@ -203,8 +204,7 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd {
   /**
    * Restoration 'strategy' that takes responsibility for creating the collection to restore to.
    *
-   * This is currently the only supported 'strategy' for backup restoration.  Though in-place restoration has been
-   * proposed and may be added soon (see SOLR-15087)
+   * @see RestoreOnExistingCollection
    */
   private class RestoreOnANewCollection {
     private int numNrtReplicas;
@@ -571,4 +571,54 @@ public class RestoreCmd implements OverseerCollectionMessageHandler.Cmd {
       }
     }
   }
+
+  /**
+   * Restoration 'strategy' that ensures the collection being restored to already exists.
+   *
+   * @see RestoreOnANewCollection
+   */
+  private class RestoreOnExistingCollection {
+
+    private RestoreOnExistingCollection(RestoreContext rc) {
+      int numShardsOfBackup = rc.backupCollectionState.getSlices().size();
+      int numShards = rc.zkStateReader.getClusterState().getCollection(rc.restoreCollectionName).getSlices().size();
+
+      if (numShardsOfBackup != numShards) {
+        String msg = String.format(Locale.ROOT, "Unable to restoring since number of shards in backup " +
+                "and specified collection does not match, numShardsOfBackup:%d numShardsOfCollection:%d", numShardsOfBackup, numShards);
+        throw new SolrException(ErrorCode.BAD_REQUEST, msg);
+      }
+    }
+
+    public void process(RestoreContext rc, @SuppressWarnings({"rawtypes"}) NamedList results) throws Exception {
+      ClusterState clusterState = rc.zkStateReader.getClusterState();
+      DocCollection restoreCollection = clusterState.getCollection(rc.restoreCollectionName);
+
+      enableReadOnly(clusterState, restoreCollection);
+      try {
+        requestReplicasToRestore(results, restoreCollection, clusterState, rc.backupProperties,
+                rc.backupPath, rc.repo, rc.shardHandler, rc.asyncId);
+      } finally {
+        disableReadOnly(clusterState, restoreCollection);
+      }
+    }
+
+    private void disableReadOnly(ClusterState clusterState, DocCollection restoreCollection) throws Exception {
+      ZkNodeProps params = new ZkNodeProps(
+              Overseer.QUEUE_OPERATION, CollectionParams.CollectionAction.MODIFYCOLLECTION.toString(),
+              ZkStateReader.COLLECTION_PROP, restoreCollection.getName(),
+              ZkStateReader.READ_ONLY, null
+      );
+      ocmh.modifyCollection(clusterState, params, new NamedList<>());
+    }
+
+    private void enableReadOnly(ClusterState clusterState, DocCollection restoreCollection) throws Exception {
+      ZkNodeProps params = new ZkNodeProps(
+              Overseer.QUEUE_OPERATION, CollectionParams.CollectionAction.MODIFYCOLLECTION.toString(),
+              ZkStateReader.COLLECTION_PROP, restoreCollection.getName(),
+              ZkStateReader.READ_ONLY, "true"
+      );
+      ocmh.modifyCollection(clusterState, params, new NamedList<>());
+    }
+  }
 }
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 15b558b..2a4d05b 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
@@ -1157,11 +1157,6 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
       req.getParams().required().check(NAME, COLLECTION_PROP);
 
       final String collectionName = SolrIdentifierValidator.validateCollectionName(req.getParams().get(COLLECTION_PROP));
-      final ClusterState clusterState = h.coreContainer.getZkController().getClusterState();
-      //We always want to restore into an collection name which doesn't  exist yet.
-      if (clusterState.hasCollection(collectionName)) {
-        throw new SolrException(ErrorCode.BAD_REQUEST, "Collection '" + collectionName + "' exists, no action taken.");
-      }
       if (h.coreContainer.getZkController().getZkStateReader().getAliases().hasAlias(collectionName)) {
         throw new SolrException(ErrorCode.BAD_REQUEST, "Collection '" + collectionName + "' is an existing alias, no action taken.");
       }
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java b/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java
index 683c802..32f52d5 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java
@@ -65,9 +65,9 @@ class RestoreCoreOp implements CoreAdminHandler.CoreAdminOp {
       // this core must be the only replica in its shard otherwise
       // we cannot guarantee consistency between replicas because when we add data (or restore index) to this replica
       Slice slice = zkController.getClusterState().getCollection(cd.getCollectionName()).getSlice(cd.getShardId());
-      if (slice.getReplicas().size() != 1) {
+      if (slice.getReplicas().size() != 1 && !core.readOnly) {
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-            "Failed to restore core=" + core.getName() + ", the core must be the only replica in its shard");
+                "Failed to restore core=" + core.getName() + ", the core must be the only replica in its shard or it must be read only");
       }
 
       RestoreCore restoreCore;
diff --git a/solr/solr-ref-guide/src/collection-management.adoc b/solr/solr-ref-guide/src/collection-management.adoc
index 118325c..a042dc5 100644
--- a/solr/solr-ref-guide/src/collection-management.adoc
+++ b/solr/solr-ref-guide/src/collection-management.adoc
@@ -1392,12 +1392,14 @@ POST http://localhost:8983/v2/collections/backups
 [[restore]]
 == RESTORE: Restore Collection
 
-Restores Solr indexes and associated configurations.
+Restores Solr indexes and associated configurations to a specified collection.
 
 `/admin/collections?action=RESTORE&name=myBackupName&location=/path/to/my/shared/drive&collection=myRestoredCollectionName`
 
-The RESTORE operation will create a collection with the specified name in the collection parameter. You cannot restore into the same collection the backup was taken from. Also the target collection should not be present at the time the API is called as Solr will create it for you.
+The RESTORE operation will replace the content of a collection with files from the specified backup.
 
+If the provided `collection` value matches an existing collection, Solr will use it for restoration, assuming it is compatible (same number of shards, etc.) with the stored backup files.
+If the provided `collection` value doesn't exist, a new collection with that name is created in a way compatible with the stored backup files.
 The collection created will be have the same number of shards and replicas as the original collection, preserving routing information, etc. Optionally, you can override some parameters documented below.
 
 While restoring, if a configset with the same name exists in ZooKeeper then Solr will reuse that, or else it will upload the backed up configset in ZooKeeper and use that.
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
index 6a6a833..ca8192b 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
@@ -45,12 +45,12 @@ import org.apache.lucene.index.IndexCommit;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
-import org.apache.lucene.util.TestUtil;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.response.CollectionAdminResponse;
 import org.apache.solr.client.solrj.response.RequestStatusState;
@@ -70,6 +70,7 @@ import org.apache.solr.core.backup.Checksum;
 import org.apache.solr.core.backup.ShardBackupId;
 import org.apache.solr.core.backup.ShardBackupMetadata;
 import org.apache.solr.core.backup.repository.BackupRepository;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -87,13 +88,11 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
 
     private static long docsSeed; // see indexDocs()
     protected static final int NUM_SHARDS = 2;//granted we sometimes shard split to get more
+    protected static final int REPL_FACTOR = 2;
     protected static final String BACKUPNAME_PREFIX = "mytestbackup";
     protected static final String BACKUP_REPO_NAME = "trackingBackupRepository";
 
     protected String testSuffix = "test1";
-    protected int replFactor;
-    protected int numTlogReplicas;
-    protected int numPullReplicas;
 
     @BeforeClass
     public static void createCluster() throws Exception {
@@ -101,6 +100,11 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
         System.setProperty("solr.directoryFactory", "solr.StandardDirectoryFactory");
     }
 
+    @Before
+    public void setUpTrackingRepo() {
+        TrackingBackupRepository.clear();
+    }
+
     /**
      * @return The name of the collection to use.
      */
@@ -114,12 +118,6 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
         this.testSuffix = testSuffix;
     }
 
-    private void randomizeReplicaTypes() {
-        replFactor = TestUtil.nextInt(random(), 1, 2);
-//    numTlogReplicas = TestUtil.nextInt(random(), 0, 1);
-//    numPullReplicas = TestUtil.nextInt(random(), 0, 1);
-    }
-
     /**
      * @return The absolute path for the backup location.
      *         Could return null.
@@ -131,7 +129,6 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
         setTestSuffix("testbackupincsimple");
         final String backupCollectionName = getCollectionName();
         final String restoreCollectionName = backupCollectionName + "_restore";
-        TrackingBackupRepository.clear();
 
         CloudSolrClient solrClient = cluster.getSolrClient();
 
@@ -166,9 +163,12 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
             log.info("Created backup with {} docs, took {}ms", numFound, timeTaken);
 
             t = System.nanoTime();
+            randomlyPrecreateRestoreCollection(restoreCollectionName, "conf1", NUM_SHARDS, 1);
             CollectionAdminRequest.restoreCollection(restoreCollectionName, backupName)
                     .setBackupId(0)
-                    .setLocation(backupLocation).setRepositoryName(BACKUP_REPO_NAME).processAndWait(solrClient, 500);
+                    .setLocation(backupLocation)
+                    .setRepositoryName(BACKUP_REPO_NAME)
+                    .processAndWait(solrClient, 500);
             timeTaken = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - t);
             log.info("Restored from backup, took {}ms", timeTaken);
             numFound = cluster.getSolrClient().query(restoreCollectionName,
@@ -178,19 +178,58 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
     }
 
     @Test
+    public void testRestoreToOriginalCollection() throws Exception {
+        setTestSuffix("testbackuprestoretooriginal");
+        final String backupCollectionName = getCollectionName();
+        final String backupName = BACKUPNAME_PREFIX + testSuffix;
+
+        // Bootstrap the backup collection with seed docs
+        CollectionAdminRequest
+                .createCollection(backupCollectionName, "conf1", NUM_SHARDS, REPL_FACTOR)
+                .setMaxShardsPerNode(-1)
+                .process(cluster.getSolrClient());
+        final int firstBatchNumDocs = indexDocs(backupCollectionName, true);
+
+        // Backup and immediately add more docs to the collection
+        try (BackupRepository repository = cluster.getJettySolrRunner(0).getCoreContainer()
+                .newBackupRepository(Optional.of(BACKUP_REPO_NAME))) {
+            final String backupLocation = repository.getBackupLocation(getBackupLocation());
+            final RequestStatusState result = CollectionAdminRequest.backupCollection(backupCollectionName, backupName)
+                    .setLocation(backupLocation)
+                    .setRepositoryName(BACKUP_REPO_NAME)
+                    .processAndWait(cluster.getSolrClient(), 10 * 1000);
+            assertEquals(RequestStatusState.COMPLETED, result);
+        }
+        final int secondBatchNumDocs = indexDocs(backupCollectionName, true);
+        final int maxDocs = secondBatchNumDocs + firstBatchNumDocs;
+        assertEquals(maxDocs, getNumDocsInCollection(backupCollectionName));
+
+        // Restore original docs and validate that doc count is correct
+        try (BackupRepository repository = cluster.getJettySolrRunner(0).getCoreContainer()
+                .newBackupRepository(Optional.of(BACKUP_REPO_NAME))) {
+            final String backupLocation = repository.getBackupLocation(getBackupLocation());
+            final RequestStatusState result = CollectionAdminRequest.restoreCollection(backupCollectionName, backupName)
+                    .setLocation(backupLocation)
+                    .setRepositoryName(BACKUP_REPO_NAME)
+                    .processAndWait(cluster.getSolrClient(), 20 * 1000);
+            assertEquals(RequestStatusState.COMPLETED, result);
+        }
+        assertEquals(firstBatchNumDocs, getNumDocsInCollection(backupCollectionName));
+
+    }
+
+    @Test
     @Slow
     @SuppressWarnings("rawtypes")
     public void testBackupIncremental() throws Exception {
-        TrackingBackupRepository.clear();
         setTestSuffix("testbackupinc");
 
-        randomizeReplicaTypes();
         CloudSolrClient solrClient = cluster.getSolrClient();
 
         CollectionAdminRequest
-                .createCollection(getCollectionName(), "conf1", NUM_SHARDS, replFactor, numTlogReplicas, numPullReplicas)
-                .setMaxShardsPerNode(-1)
-                .process(solrClient);
+            .createCollection(getCollectionName(), "conf1", NUM_SHARDS, REPL_FACTOR)
+            .setMaxShardsPerNode(-1)
+            .process(solrClient);
 
         indexDocs(getCollectionName(), false);
 
@@ -342,8 +381,11 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
 
         String restoreCollectionName = getCollectionName() + "_restored";
 
+        randomlyPrecreateRestoreCollection(restoreCollectionName, "conf1", NUM_SHARDS, REPL_FACTOR);
         CollectionAdminRequest.restoreCollection(restoreCollectionName, backupName)
-                .setLocation(backupLocation).setRepositoryName(BACKUP_REPO_NAME).process(solrClient);
+                .setLocation(backupLocation)
+                .setRepositoryName(BACKUP_REPO_NAME)
+                .process(solrClient);
 
         AbstractDistribZkTestBase.waitForRecoveriesToFinish(
                 restoreCollectionName, cluster.getSolrClient().getZkStateReader(), log.isDebugEnabled(), true, 30);
@@ -381,6 +423,19 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase {
         return numDocs;
     }
 
+    private void randomlyPrecreateRestoreCollection(String restoreCollectionName, String configName, int numShards, int numReplicas) throws Exception {
+        if (random().nextBoolean()) {
+            CollectionAdminRequest.createCollection(restoreCollectionName, configName, numShards, numReplicas)
+                    .setMaxShardsPerNode(-1)
+                    .process(cluster.getSolrClient());
+            cluster.waitForActiveCollection(restoreCollectionName, numShards, numShards*numReplicas);
+        }
+    }
+
+    private long getNumDocsInCollection(String collectionName) throws Exception {
+        return new QueryRequest(new SolrQuery("*:*")).process(cluster.getSolrClient(), collectionName).getResults().getNumFound();
+    }
+
     private class IncrementalBackupVerifier {
         private BackupRepository repository;
         private URI backupURI;