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 2023/05/08 15:14:18 UTC

[solr] branch branch_9x updated: SOLR-16394: Tweak v2 restore-coll to be more REST-ful (#1592)

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 a0ddc123629 SOLR-16394: Tweak v2 restore-coll to be more REST-ful (#1592)
a0ddc123629 is described below

commit a0ddc123629688d49b9c0ec0ffd90e100aa9951e
Author: Jason Gerlowski <ge...@apache.org>
AuthorDate: Fri Apr 28 08:55:26 2023 -0400

    SOLR-16394: Tweak v2 restore-coll to be more REST-ful (#1592)
    
    This commit tweaks the v2 binding for our "restore collection" API to be
    more intuitive for users.  Restore now lives at:
    
      - POST /api/backups/backupName/restore {...}
    
    This commit also switches it over to using the JAX-RS framework.
---
 solr/CHANGES.txt                                   |   3 +
 .../solr/handler/admin/CollectionsHandler.java     |  87 +-------
 .../handler/admin/api/CreateCollectionAPI.java     |   3 +-
 .../admin/api/CreateCollectionBackupAPI.java       |   6 +-
 .../handler/admin/api/RestoreCollectionAPI.java    | 239 ++++++++++++++++++---
 .../org/apache/solr/util/tracing/TraceUtils.java   |   2 +-
 .../handler/admin/V2CollectionsAPIMappingTest.java |  97 ---------
 .../admin/api/RestoreCollectionAPITest.java        | 202 +++++++++++++++++
 .../pages/collection-management.adoc               |  39 ++--
 .../request/beans/RestoreCollectionPayload.java    |  45 ----
 10 files changed, 443 insertions(+), 280 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 24ba6dcfd21..2ccd8929ea7 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -79,6 +79,9 @@ Improvements
   to "type", and the routers themselves are now always grouped into a list. Additionally the v2 API has moved to the new path
   `POST /api/aliases`. (Jason Gerlowski)
 
+* SOLR-16394: The v2 "restore" API has been tweaked to be more intuitive.  The top-level "restore-collection" command
+  specifier has been removed, and the API now lives at the new path `POST /api/backups/backupName/restore`. (Jason Gerlowski)
+
 Optimizations
 ---------------------
 
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 6d39c61f656..4ddf73ae24c 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
@@ -23,7 +23,6 @@ import static org.apache.solr.client.solrj.response.RequestStatusState.RUNNING;
 import static org.apache.solr.client.solrj.response.RequestStatusState.SUBMITTED;
 import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
 import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET;
-import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET_EMPTY;
 import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.CREATE_NODE_SET_SHUFFLE;
 import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.NUM_SLICES;
 import static org.apache.solr.cloud.api.collections.CollectionHandlingUtils.ONLY_ACTIVE_NODES;
@@ -42,7 +41,6 @@ import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_TYPE;
 import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS;
 import static org.apache.solr.common.params.CollectionAdminParams.COLLECTION;
-import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
 import static org.apache.solr.common.params.CollectionAdminParams.COUNT_PROP;
 import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
 import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES;
@@ -1206,86 +1204,9 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
     RESTORE_OP(
         RESTORE,
         (req, rsp, h) -> {
-          req.getParams().required().check(NAME, COLLECTION_PROP);
-
-          final String collectionName =
-              SolrIdentifierValidator.validateCollectionName(req.getParams().get(COLLECTION_PROP));
-          if (h.coreContainer
-              .getZkController()
-              .getZkStateReader()
-              .getAliases()
-              .hasAlias(collectionName)) {
-            throw new SolrException(
-                ErrorCode.BAD_REQUEST,
-                "Collection '" + collectionName + "' is an existing alias, no action taken.");
-          }
-
-          final CoreContainer cc = h.coreContainer;
-          final String repo = req.getParams().get(CoreAdminParams.BACKUP_REPOSITORY);
-          final BackupRepository repository = cc.newBackupRepository(repo);
-
-          String location =
-              repository.getBackupLocation(req.getParams().get(CoreAdminParams.BACKUP_LOCATION));
-          if (location == null) {
-            // Refresh the cluster property file to make sure the value set for location is the
-            // latest. Check if the location is specified in the cluster property.
-            location =
-                new ClusterProperties(h.coreContainer.getZkController().getZkClient())
-                    .getClusterProperty("location", null);
-            if (location == null) {
-              throw new SolrException(
-                  ErrorCode.BAD_REQUEST,
-                  "'location' is not specified as a query"
-                      + " parameter or as a default repository property or as a cluster property.");
-            }
-          }
-
-          // Check if the specified location is valid for this repository.
-          final URI uri = repository.createDirectoryURI(location);
-          try {
-            if (!repository.exists(uri)) {
-              throw new SolrException(
-                  ErrorCode.SERVER_ERROR, "specified location " + uri + " does not exist.");
-            }
-          } catch (IOException ex) {
-            throw new SolrException(
-                ErrorCode.SERVER_ERROR,
-                "Failed to check the existence of " + uri + ". Is it valid?",
-                ex);
-          }
-
-          final String createNodeArg = req.getParams().get(CREATE_NODE_SET);
-          if (CREATE_NODE_SET_EMPTY.equals(createNodeArg)) {
-            throw new SolrException(
-                SolrException.ErrorCode.BAD_REQUEST,
-                "Cannot restore with a CREATE_NODE_SET of CREATE_NODE_SET_EMPTY.");
-          }
-          if (req.getParams().get(NRT_REPLICAS) != null
-              && req.getParams().get(REPLICATION_FACTOR) != null) {
-            throw new SolrException(
-                SolrException.ErrorCode.BAD_REQUEST,
-                "Cannot set both replicationFactor and nrtReplicas as they mean the same thing");
-          }
-
-          final Map<String, Object> params = copy(req.getParams(), null, NAME, COLLECTION_PROP);
-          params.put(CoreAdminParams.BACKUP_LOCATION, location);
-          if (repo != null) {
-            params.put(CoreAdminParams.BACKUP_REPOSITORY, repo);
-          }
-          // from CREATE_OP:
-          copy(
-              req.getParams(),
-              params,
-              COLL_CONF,
-              REPLICATION_FACTOR,
-              NRT_REPLICAS,
-              TLOG_REPLICAS,
-              PULL_REPLICAS,
-              CREATE_NODE_SET,
-              CREATE_NODE_SET_SHUFFLE,
-              BACKUP_ID);
-          copyPropertiesWithPrefix(req.getParams(), params, PROPERTY_PREFIX);
-          return params;
+          final var response = RestoreCollectionAPI.invokeFromV1Params(req, rsp, h.coreContainer);
+          V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, response);
+          return null;
         }),
     INSTALLSHARDDATA_OP(
         INSTALLSHARDDATA,
@@ -1813,6 +1734,7 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
         InstallShardDataAPI.class,
         ListCollectionsAPI.class,
         ReplaceNodeAPI.class,
+        RestoreCollectionAPI.class,
         CollectionPropertyAPI.class,
         DeleteNodeAPI.class,
         ListAliasesAPI.class,
@@ -1825,7 +1747,6 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
   @Override
   public Collection<Api> getApis() {
     final List<Api> apis = new ArrayList<>();
-    apis.addAll(AnnotatedApi.getApis(new RestoreCollectionAPI(this)));
     apis.addAll(AnnotatedApi.getApis(new SplitShardAPI(this)));
     apis.addAll(AnnotatedApi.getApis(new CreateShardAPI(this)));
     apis.addAll(AnnotatedApi.getApis(new AddReplicaAPI(this)));
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionAPI.java
index 94ccfb9f183..1ff34e69ff7 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionAPI.java
@@ -187,7 +187,8 @@ public class CreateCollectionAPI extends AdminAPIBase {
     rawProperties.put(NAME, reqBody.name);
     rawProperties.put(COLL_CONF, reqBody.config);
     rawProperties.put(NUM_SLICES, reqBody.numShards);
-    rawProperties.put(CREATE_NODE_SET_SHUFFLE, reqBody.shuffleNodes);
+    if (reqBody.shuffleNodes != null)
+      rawProperties.put(CREATE_NODE_SET_SHUFFLE, reqBody.shuffleNodes);
     if (CollectionUtil.isNotEmpty(reqBody.shardNames))
       rawProperties.put(SHARDS_PROP, String.join(",", reqBody.shardNames));
     rawProperties.put(PULL_REPLICAS, reqBody.pullReplicas);
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java
index cfb835ff3ff..52e6108183e 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java
@@ -110,7 +110,7 @@ public class CreateCollectionBackupAPI extends AdminAPIBase {
             collectionName, Boolean.TRUE.equals(requestBody.followAliases));
 
     final BackupRepository repository = coreContainer.newBackupRepository(requestBody.repository);
-    requestBody.location = getLocation(repository, requestBody.location);
+    requestBody.location = getLocation(coreContainer, repository, requestBody.location);
     if (requestBody.incremental == null) {
       requestBody.incremental = Boolean.TRUE;
     }
@@ -198,7 +198,9 @@ public class CreateCollectionBackupAPI extends AdminAPIBase {
     return createBackupApi.createCollectionBackup(collectionName, backupName, requestBody);
   }
 
-  private String getLocation(BackupRepository repository, String location) throws IOException {
+  public static String getLocation(
+      CoreContainer coreContainer, BackupRepository repository, String location)
+      throws IOException {
     location = repository.getBackupLocation(location);
     if (location != null) {
       return location;
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCollectionAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCollectionAPI.java
index cdefb7b251f..f61cc312882 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCollectionAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCollectionAPI.java
@@ -17,52 +17,227 @@
 
 package org.apache.solr.handler.admin.api;
 
-import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
-import static org.apache.solr.common.params.CommonParams.ACTION;
+import static org.apache.solr.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.client.solrj.request.beans.V2ApiConstants.CREATE_COLLECTION_KEY;
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
+import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_SHUFFLE_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.NRT_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX;
+import static org.apache.solr.common.params.CollectionAdminParams.PULL_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.REPLICATION_FACTOR;
+import static org.apache.solr.common.params.CollectionAdminParams.TLOG_REPLICAS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
 import static org.apache.solr.common.params.CommonParams.NAME;
-import static org.apache.solr.handler.ClusterAPI.wrapParams;
-import static org.apache.solr.handler.admin.api.CreateCollectionAPI.CreateCollectionRequestBody.convertV2CreateCollectionMapToV1ParamMap;
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_ID;
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_LOCATION;
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_REPOSITORY;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
 import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.io.IOException;
+import java.net.URI;
 import java.util.HashMap;
 import java.util.Map;
-import org.apache.solr.api.Command;
-import org.apache.solr.api.EndPoint;
-import org.apache.solr.api.PayloadObj;
-import org.apache.solr.client.solrj.request.beans.RestoreCollectionPayload;
-import org.apache.solr.client.solrj.request.beans.V2ApiConstants;
+import java.util.Set;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.client.solrj.util.SolrIdentifierValidator;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.backup.repository.BackupRepository;
 import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.jersey.SubResponseAccumulatingJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
 
-@EndPoint(
-    path = {"/backups/{backupName}"},
-    method = POST,
-    permission = COLL_EDIT_PERM)
-public class RestoreCollectionAPI {
+/**
+ * V2 API for restoring data into a collection
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=RESTORE command.
+ */
+@Path("/backups/{backupName}/restore")
+public class RestoreCollectionAPI extends AdminAPIBase {
+
+  private static final Set<String> CREATE_PARAM_ALLOWLIST =
+      Set.of(
+          COLL_CONF,
+          REPLICATION_FACTOR,
+          NRT_REPLICAS,
+          TLOG_REPLICAS,
+          PULL_REPLICAS,
+          CREATE_NODE_SET_PARAM,
+          CREATE_NODE_SET_SHUFFLE_PARAM);
+
+  @Inject
+  public RestoreCollectionAPI(
+      CoreContainer coreContainer,
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse) {
+    super(coreContainer, solrQueryRequest, solrQueryResponse);
+  }
+
+  @POST
+  @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+  @PermissionName(COLL_EDIT_PERM)
+  public SubResponseAccumulatingJerseyResponse restoreCollection(
+      @PathParam("backupName") String backupName, RestoreCollectionRequestBody requestBody)
+      throws Exception {
+    final var response = instantiateJerseyResponse(SubResponseAccumulatingJerseyResponse.class);
+
+    if (requestBody == null) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Missing required request body");
+    }
+    if (requestBody.collection == null) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST, "Required parameter 'collection' missing");
+    }
+    if (backupName == null) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST, "Required parameter 'backupName' missing");
+    }
+
+    final String collectionName = requestBody.collection;
+    recordCollectionForLogAndTracing(collectionName, solrQueryRequest);
+    SolrIdentifierValidator.validateCollectionName(collectionName);
+    if (coreContainer.getAliases().hasAlias(collectionName)) {
+      throw new SolrException(
+          SolrException.ErrorCode.BAD_REQUEST,
+          "Collection '" + collectionName + "' is an existing alias, no action taken.");
+    }
+
+    final BackupRepository repository = coreContainer.newBackupRepository(requestBody.repository);
+    requestBody.location =
+        CreateCollectionBackupAPI.getLocation(coreContainer, repository, requestBody.location);
+
+    // Check if the specified location is valid for this repository.
+    final URI uri = repository.createDirectoryURI(requestBody.location);
+    try {
+      if (!repository.exists(uri)) {
+        throw new SolrException(
+            SolrException.ErrorCode.SERVER_ERROR, "specified location " + uri + " does not exist.");
+      }
+    } catch (IOException ex) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR,
+          "Failed to check the existence of " + uri + ". Is it valid?",
+          ex);
+    }
+
+    final var createRequestBody = requestBody.createCollectionParams;
+    if (createRequestBody != null) {
+      CreateCollectionAPI.populateDefaultsIfNecessary(coreContainer, createRequestBody);
+      createRequestBody.validate();
+      if (Boolean.FALSE.equals(createRequestBody.createReplicas)) {
+        throw new SolrException(
+            SolrException.ErrorCode.BAD_REQUEST,
+            "Replica-creation cannot be disabled for collections created by a restore operation.");
+      }
+    }
+
+    final ZkNodeProps remoteMessage = createRemoteMessage(backupName, requestBody);
+    final SolrResponse remoteResponse =
+        CollectionsHandler.submitCollectionApiCommand(
+            coreContainer,
+            coreContainer.getDistributedCollectionCommandRunner(),
+            remoteMessage,
+            CollectionParams.CollectionAction.RESTORE,
+            DEFAULT_COLLECTION_OP_TIMEOUT);
 
-  public static final String V2_RESTORE_CMD = "restore-collection";
+    if (remoteResponse.getException() != null) {
+      throw remoteResponse.getException();
+    }
 
-  private final CollectionsHandler collectionsHandler;
+    // Values fetched from remoteResponse may be null
+    response.successfulSubResponsesByNodeName = remoteResponse.getResponse().get("success");
+    response.failedSubResponsesByNodeName = remoteResponse.getResponse().get("failure");
 
-  public RestoreCollectionAPI(CollectionsHandler collectionsHandler) {
-    this.collectionsHandler = collectionsHandler;
+    return response;
   }
 
-  @Command(name = V2_RESTORE_CMD)
-  @SuppressWarnings("unchecked")
-  public void restoreBackup(PayloadObj<RestoreCollectionPayload> obj) throws Exception {
-    final RestoreCollectionPayload v2Body = obj.get();
-    final Map<String, Object> v1Params = v2Body.toMap(new HashMap<>());
-
-    v1Params.put(ACTION, CollectionParams.CollectionAction.RESTORE.toLower());
-    v1Params.put(NAME, obj.getRequest().getPathTemplateValues().get("backupName"));
-    if (v2Body.createCollectionParams != null && !v2Body.createCollectionParams.isEmpty()) {
-      final Map<String, Object> createCollParams =
-          (Map<String, Object>) v1Params.remove(V2ApiConstants.CREATE_COLLECTION_KEY);
-      convertV2CreateCollectionMapToV1ParamMap(createCollParams);
-      v1Params.putAll(createCollParams);
+  public static ZkNodeProps createRemoteMessage(
+      String backupName, RestoreCollectionRequestBody requestBody) {
+    final Map<String, Object> remoteMessage = requestBody.toMap(new HashMap<>());
+
+    // If the RESTORE is setup to create a new collection, copy those parameters first
+    final var createReqBody = requestBody.createCollectionParams;
+    if (createReqBody != null) {
+      // RESTORE only supports a subset of collection-creation params, so filter by those when
+      // constructing the remote message
+      remoteMessage.remove("create-collection");
+      CreateCollectionAPI.createRemoteMessage(createReqBody).getProperties().entrySet().stream()
+          .filter(
+              e ->
+                  CREATE_PARAM_ALLOWLIST.contains(e.getKey())
+                      || e.getKey().startsWith(PROPERTY_PREFIX))
+          .forEach(e -> remoteMessage.put(e.getKey(), e.getValue()));
     }
 
-    collectionsHandler.handleRequestBody(wrapParams(obj.getRequest(), v1Params), obj.getResponse());
+    // Copy restore-specific parameters
+    remoteMessage.put(QUEUE_OPERATION, CollectionParams.CollectionAction.RESTORE.toLower());
+    remoteMessage.put(COLLECTION_PROP, requestBody.collection);
+    remoteMessage.put(NAME, backupName);
+    remoteMessage.put(BACKUP_LOCATION, requestBody.location);
+    if (requestBody.backupId != null) remoteMessage.put(BACKUP_ID, requestBody.backupId);
+    if (requestBody.repository != null)
+      remoteMessage.put(BACKUP_REPOSITORY, requestBody.repository);
+    return new ZkNodeProps(remoteMessage);
+  }
+
+  public static SolrJerseyResponse invokeFromV1Params(
+      SolrQueryRequest solrQueryRequest,
+      SolrQueryResponse solrQueryResponse,
+      CoreContainer coreContainer)
+      throws Exception {
+    final var params = solrQueryRequest.getParams();
+    params.required().check(NAME, COLLECTION_PROP);
+    final String backupName = params.get(NAME);
+    final var requestBody = RestoreCollectionRequestBody.fromV1Params(params);
+
+    final var restoreApi =
+        new RestoreCollectionAPI(coreContainer, solrQueryRequest, solrQueryResponse);
+    return restoreApi.restoreCollection(backupName, requestBody);
+  }
+
+  /** Request body for the v2 "restore collection" API. */
+  public static class RestoreCollectionRequestBody implements JacksonReflectMapWriter {
+    @JsonProperty(required = true)
+    public String collection;
+
+    @JsonProperty public String location;
+    @JsonProperty public String repository;
+    @JsonProperty public Integer backupId;
+
+    @JsonProperty(CREATE_COLLECTION_KEY)
+    public CreateCollectionAPI.CreateCollectionRequestBody createCollectionParams;
+
+    @JsonProperty public String async;
+
+    public static RestoreCollectionRequestBody fromV1Params(SolrParams solrParams) {
+      final var restoreBody = new RestoreCollectionRequestBody();
+      restoreBody.collection = solrParams.get(COLLECTION_PROP);
+      restoreBody.location = solrParams.get(BACKUP_LOCATION);
+      restoreBody.repository = solrParams.get(BACKUP_REPOSITORY);
+      restoreBody.backupId = solrParams.getInt(BACKUP_ID);
+      restoreBody.async = solrParams.get(ASYNC);
+
+      restoreBody.createCollectionParams =
+          CreateCollectionAPI.CreateCollectionRequestBody.fromV1Params(solrParams, false);
+
+      return restoreBody;
+    }
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
index 5a38528a7a0..c7d2f0f0fd9 100644
--- a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
@@ -26,7 +26,7 @@ import org.apache.solr.request.SolrQueryRequest;
 public class TraceUtils {
 
   public static void setDbInstance(SolrQueryRequest req, String coreOrColl) {
-    if (coreOrColl != null) {
+    if (req != null && coreOrColl != null) {
       ifNotNoop(req.getSpan(), (span) -> span.setTag(Tags.DB_INSTANCE, coreOrColl));
     }
   }
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/V2CollectionsAPIMappingTest.java b/solr/core/src/test/org/apache/solr/handler/admin/V2CollectionsAPIMappingTest.java
deleted file mode 100644
index caa312e9d0a..00000000000
--- a/solr/core/src/test/org/apache/solr/handler/admin/V2CollectionsAPIMappingTest.java
+++ /dev/null
@@ -1,97 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.handler.admin;
-
-import static org.apache.solr.common.params.CommonParams.ACTION;
-
-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.CommonAdminParams;
-import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.params.CoreAdminParams;
-import org.apache.solr.common.params.SolrParams;
-import org.apache.solr.core.backup.BackupManager;
-import org.apache.solr.handler.admin.api.RestoreCollectionAPI;
-import org.junit.Test;
-
-/**
- * Unit tests for the "create collection", "create alias" and "restore collection" v2 APIs.
- *
- * <p>This test bears many similarities to {@link TestCollectionAPIs} which appears to test the
- * mappings indirectly by checking message sent to the ZK overseer (which is similar, but not
- * identical to the v1 param list). If there's no particular benefit to testing the mappings in this
- * way (there very well may be), then we should combine these two test classes at some point in the
- * future using the simpler approach here.
- *
- * <p>Note that the V2 requests made by these tests are not necessarily semantically valid. They
- * shouldn't be taken as examples. In several instances, mutually exclusive JSON parameters are
- * provided. This is done to exercise conversion of all parameters, even if particular combinations
- * are never expected in the same request.
- */
-public class V2CollectionsAPIMappingTest extends V2ApiMappingTest<CollectionsHandler> {
-
-  @Override
-  public void populateApiBag() {
-    apiBag.registerObject(new RestoreCollectionAPI(getRequestHandler()));
-  }
-
-  @Override
-  public CollectionsHandler createUnderlyingRequestHandler() {
-    return createMock(CollectionsHandler.class);
-  }
-
-  @Override
-  public boolean isCoreSpecific() {
-    return false;
-  }
-
-  @Test
-  public void testRestoreAllProperties() throws Exception {
-    final SolrParams v1Params =
-        captureConvertedV1Params(
-            "/backups/backupName",
-            "POST",
-            "{'restore-collection': {"
-                + "'collection': 'collectionName', "
-                + "'location': '/some/location/uri', "
-                + "'repository': 'someRepository', "
-                + "'backupId': 123, "
-                + "'async': 'requestTrackingId', "
-                + "'create-collection': {"
-                + "     'numShards': 1, "
-                + "     'properties': {'foo': 'bar', 'foo2': 'bar2'}, "
-                + "     'replicationFactor': 3 "
-                + "}"
-                + "}}");
-
-    assertEquals(CollectionParams.CollectionAction.RESTORE.lowerName, v1Params.get(ACTION));
-    assertEquals("backupName", v1Params.get(CommonParams.NAME));
-    assertEquals("collectionName", v1Params.get(BackupManager.COLLECTION_NAME_PROP));
-    assertEquals("/some/location/uri", v1Params.get(CoreAdminParams.BACKUP_LOCATION));
-    assertEquals("someRepository", v1Params.get(CoreAdminParams.BACKUP_REPOSITORY));
-    assertEquals(123, v1Params.getPrimitiveInt(CoreAdminParams.BACKUP_ID));
-    assertEquals("requestTrackingId", v1Params.get(CommonAdminParams.ASYNC));
-    // NOTE: Unlike other v2 APIs that have a nested object for collection-creation params,
-    // the restore API's v1 equivalent for these properties doesn't have a "create-collection."
-    // prefix.
-    assertEquals(1, v1Params.getPrimitiveInt(CollectionAdminParams.NUM_SHARDS));
-    assertEquals("bar", v1Params.get("property.foo"));
-    assertEquals("bar2", v1Params.get("property.foo2"));
-    assertEquals(3, v1Params.getPrimitiveInt(ZkStateReader.REPLICATION_FACTOR));
-  }
-}
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/RestoreCollectionAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/RestoreCollectionAPITest.java
new file mode 100644
index 00000000000..52321b3f3c8
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/handler/admin/api/RestoreCollectionAPITest.java
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.handler.admin.api;
+
+import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionAdminParams.COLLECTION;
+import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF;
+import static org.apache.solr.common.params.CollectionAdminParams.CREATE_NODE_SET_PARAM;
+import static org.apache.solr.common.params.CollectionAdminParams.NRT_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.PULL_REPLICAS;
+import static org.apache.solr.common.params.CollectionAdminParams.REPLICATION_FACTOR;
+import static org.apache.solr.common.params.CollectionAdminParams.TLOG_REPLICAS;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_ID;
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_LOCATION;
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_REPOSITORY;
+import static org.apache.solr.common.params.CoreAdminParams.NAME;
+import static org.hamcrest.Matchers.containsString;
+
+import java.util.List;
+import java.util.Map;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.hamcrest.MatcherAssert;
+import org.junit.Test;
+
+/** Unit tests for {@link RestoreCollectionAPI} */
+public class RestoreCollectionAPITest extends SolrTestCaseJ4 {
+
+  @Test
+  public void testReportsErrorIfBackupNameMissing() {
+    final var requestBody = new RestoreCollectionAPI.RestoreCollectionRequestBody();
+    requestBody.collection = "someCollection";
+    final SolrException thrown =
+        expectThrows(
+            SolrException.class,
+            () -> {
+              final var api = new RestoreCollectionAPI(null, null, null);
+              api.restoreCollection(null, requestBody);
+            });
+
+    assertEquals(400, thrown.code());
+    assertEquals("Required parameter 'backupName' missing", thrown.getMessage());
+  }
+
+  @Test
+  public void testReportsErrorIfRequestBodyMissing() {
+    final SolrException thrown =
+        expectThrows(
+            SolrException.class,
+            () -> {
+              final var api = new RestoreCollectionAPI(null, null, null);
+              api.restoreCollection("someBackupName", null);
+            });
+
+    assertEquals(400, thrown.code());
+    assertEquals("Missing required request body", thrown.getMessage());
+  }
+
+  @Test
+  public void testReportsErrorIfCollectionNameMissing() {
+    // No 'collection' set on the requestBody
+    final var requestBody = new RestoreCollectionAPI.RestoreCollectionRequestBody();
+    final SolrException thrown =
+        expectThrows(
+            SolrException.class,
+            () -> {
+              final var api = new RestoreCollectionAPI(null, null, null);
+              api.restoreCollection("someBackupName", requestBody);
+            });
+
+    assertEquals(400, thrown.code());
+    assertEquals("Required parameter 'collection' missing", thrown.getMessage());
+  }
+
+  @Test
+  public void testReportsErrorIfProvidedCollectionNameIsInvalid() {
+    final var requestBody = new RestoreCollectionAPI.RestoreCollectionRequestBody();
+    requestBody.collection = "invalid$collection@name";
+    final SolrException thrown =
+        expectThrows(
+            SolrException.class,
+            () -> {
+              final var api = new RestoreCollectionAPI(null, null, null);
+              api.restoreCollection("someBackupName", requestBody);
+            });
+
+    assertEquals(400, thrown.code());
+    MatcherAssert.assertThat(
+        thrown.getMessage(), containsString("Invalid collection: [invalid$collection@name]"));
+  }
+
+  @Test
+  public void testCreatesValidRemoteMessageForExistingCollectionRestore() {
+    final var requestBody = new RestoreCollectionAPI.RestoreCollectionRequestBody();
+    requestBody.collection = "someCollectionName";
+    requestBody.location = "/some/location/path";
+    requestBody.backupId = 123;
+    requestBody.repository = "someRepositoryName";
+    requestBody.async = "someAsyncId";
+
+    final var remoteMessage =
+        RestoreCollectionAPI.createRemoteMessage("someBackupName", requestBody).getProperties();
+
+    assertEquals(7, remoteMessage.size());
+    assertEquals("restore", remoteMessage.get(QUEUE_OPERATION));
+    assertEquals("someCollectionName", remoteMessage.get(COLLECTION));
+    assertEquals("/some/location/path", remoteMessage.get(BACKUP_LOCATION));
+    assertEquals(Integer.valueOf(123), remoteMessage.get(BACKUP_ID));
+    assertEquals("someRepositoryName", remoteMessage.get(BACKUP_REPOSITORY));
+    assertEquals("someAsyncId", remoteMessage.get(ASYNC));
+    assertEquals("someBackupName", remoteMessage.get(NAME));
+  }
+
+  @Test
+  public void testCreatesValidRemoteMessageForNewCollectionRestore() {
+    final var requestBody = new RestoreCollectionAPI.RestoreCollectionRequestBody();
+    requestBody.collection = "someCollectionName";
+    requestBody.location = "/some/location/path";
+    requestBody.backupId = 123;
+    requestBody.repository = "someRepositoryName";
+    requestBody.async = "someAsyncId";
+    final var createParams = new CreateCollectionAPI.CreateCollectionRequestBody();
+    requestBody.createCollectionParams = createParams;
+    createParams.config = "someConfig";
+    createParams.nrtReplicas = 123;
+    createParams.tlogReplicas = 456;
+    createParams.pullReplicas = 789;
+    createParams.nodeSet = List.of("node1", "node2");
+    createParams.properties = Map.of("foo", "bar");
+
+    final var remoteMessage =
+        RestoreCollectionAPI.createRemoteMessage("someBackupName", requestBody).getProperties();
+
+    assertEquals(14, remoteMessage.size());
+    assertEquals("restore", remoteMessage.get(QUEUE_OPERATION));
+    assertEquals("someCollectionName", remoteMessage.get(COLLECTION));
+    assertEquals("/some/location/path", remoteMessage.get(BACKUP_LOCATION));
+    assertEquals(Integer.valueOf(123), remoteMessage.get(BACKUP_ID));
+    assertEquals("someRepositoryName", remoteMessage.get(BACKUP_REPOSITORY));
+    assertEquals("someAsyncId", remoteMessage.get(ASYNC));
+    assertEquals("someBackupName", remoteMessage.get(NAME));
+    assertEquals("someConfig", remoteMessage.get(COLL_CONF));
+    assertEquals(Integer.valueOf(123), remoteMessage.get(NRT_REPLICAS));
+    assertEquals(Integer.valueOf(123), remoteMessage.get(REPLICATION_FACTOR));
+    assertEquals(Integer.valueOf(456), remoteMessage.get(TLOG_REPLICAS));
+    assertEquals(Integer.valueOf(789), remoteMessage.get(PULL_REPLICAS));
+    assertEquals("node1,node2", remoteMessage.get(CREATE_NODE_SET_PARAM));
+    assertEquals("bar", remoteMessage.get("property.foo"));
+  }
+
+  @Test
+  public void testCanConvertV1ParamsIntoV2RequestBody() {
+    final var v1Params = new ModifiableSolrParams();
+    v1Params.add("name", "someBackupName");
+    v1Params.add("collection", "someCollectionName");
+    v1Params.add("location", "/some/location/str");
+    v1Params.add("repository", "someRepositoryName");
+    v1Params.add("backupId", "123");
+    v1Params.add("async", "someAsyncId");
+    // Supported coll-creation params
+    v1Params.add("collection.configName", "someConfig");
+    v1Params.add("nrtReplicas", "123");
+    v1Params.add("property.foo", "bar");
+    v1Params.add("createNodeSet", "node1,node2");
+    v1Params.add("createNodeSet.shuffle", "false");
+
+    final var requestBody =
+        RestoreCollectionAPI.RestoreCollectionRequestBody.fromV1Params(v1Params);
+
+    assertEquals("someCollectionName", requestBody.collection);
+    assertEquals("/some/location/str", requestBody.location);
+    assertEquals("someRepositoryName", requestBody.repository);
+    assertEquals(Integer.valueOf(123), requestBody.backupId);
+    assertEquals("someAsyncId", requestBody.async);
+    assertNotNull(requestBody.createCollectionParams);
+    final var createParams = requestBody.createCollectionParams;
+    assertEquals("someConfig", createParams.config);
+    assertEquals(Integer.valueOf(123), createParams.nrtReplicas);
+    assertNotNull(createParams.properties);
+    assertEquals(1, createParams.properties.size());
+    assertEquals("bar", createParams.properties.get("foo"));
+    assertEquals(List.of("node1", "node2"), createParams.nodeSet);
+    assertEquals(Boolean.FALSE, createParams.shuffleNodes);
+  }
+}
diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc
index 2110a8fc2c4..a8e5592b964 100644
--- a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc
+++ b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc
@@ -1878,7 +1878,7 @@ Restores Solr indexes and associated configurations to a specified collection.
 
 [source,bash]
 ----
-http://localhost:8983/solr/admin/collections?action=RESTORE&name=techproducts_backup&location=file:///path/to/my/shared/drive&collection=techproducts_v3
+curl -X GET http://localhost:8983/solr/admin/collections?action=RESTORE&name=techproducts_backup&location=file:///path/to/my/shared/drive&collection=techproducts_v3&nrtReplicas=2&createNodeSet=node1,node2&property.foo=bar
 
 ----
 ====
@@ -1887,17 +1887,19 @@ http://localhost:8983/solr/admin/collections?action=RESTORE&name=techproducts_ba
 ====
 [.tab-label]*V2 API*
 
-With the v2 API, the `restore-collection` command is provided as part of the JSON data that contains the required parameters:
-
 [source,bash]
 ----
-curl -X POST http://localhost:8983/api/collections -H 'Content-Type: application/json' -d '
+curl -X POST http://localhost:8983/api/backups/techproducts_backup/restore -H 'Content-Type: application/json' -d '
   {
-    "restore-collection": {
-      "name": "techproducts_backup",
       "collection": "techproducts_v3",
-      "location": "file:///path/to/my/shared/drive"
-    }
+      "location": "file:///path/to/my/shared/drive",
+      "create-collection": {
+        "nrtReplicas": 2,
+        "nodeSet": ["node1", "node2"],
+        "properties": {
+          "foo": "bar"
+        }
+      }
   }
 '
 ----
@@ -1908,8 +1910,8 @@ The RESTORE operation will replace the content of a collection with files from t
 
 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.
+The collection created will have the same number of shards and replicas as the original collection, preserving routing information, etc.
+Optionally, you can override some parameters (see 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.
 
@@ -1935,6 +1937,8 @@ s|Required |Default: none
 |===
 +
 The name of the existing backup that you want to restore.
+This parameter is required.
+Provided as a query-parameter for v1 requests, and in the URL path for v2 requests.
 
 `location`::
 +
@@ -1943,7 +1947,7 @@ The name of the existing backup that you want to restore.
 |Optional |Default: none
 |===
 +
-The location on a shared drive for the RESTORE command to read from.
+The location within a backup repository for the RESTORE command to read from.
 Alternately it can be set as a xref:cluster-node-management.adoc#clusterprop[cluster property].+
 +
 This must be the same value as was given as the `location` option when <<#backup,creating the backup>>.
@@ -1980,16 +1984,13 @@ Backup locations can hold multiple backups of the same collection.
 This parameter allows users to choose which of those backups should be used to restore from.
 If not specified the most recent backup point is used.
 
-There are also optional parameters that determine the target collection layout.
-The following parameters are currently supported (described in detail in the <<create,CREATE collection>> section):
-`createNodeSet`, `createNodeSet.shuffle`.
-
-Note: for `createNodeSet` the special value of `EMPTY` is not allowed with this command.
 
 *Overridable Parameters*
 
-Additionally, there are several parameters that may have been set on the original collection that can be overridden when restoring the backup (described in detail in the <<create,CREATE collection>> section):
-`collection.configName`, `replicationFactor`, `nrtReplicas`, `tlogReplicas`, `pullReplicas`, `property._name_=_value_`.
+Additionally, users may provide a number of collection creation parameters to be used when the collection being restored to doesn't already exist.
+These include: `collection.configName`, `createNodeSet` (`EMPTY` not supported), `createNodeSet.shuffle`, `nrtReplicas`, `property._name_=_value_` (i.e. arbitrary collection properties), `pullReplicas`, `replicationFactor`, and `tlogReplicas`.
+
+See the <<create,collection creation>> documentation for more information on each of these parameters and details on their v1 or v2 specific syntaxes.
 
 [[deletebackup]]
 == DELETEBACKUP: Delete backup files from the remote repository
@@ -2325,7 +2326,7 @@ curl -X POST http://localhost:8983/api/collections/techproducts/snapshots/snapsh
     "status": 0,
     "QTime": 214
   },
-  "requestid": "someAsyncId"
+  "requestid": "someAsyncId",
   "collection": "techproducts",
   "snapshot": "snapshot0",
   "followAliases": true
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/RestoreCollectionPayload.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/RestoreCollectionPayload.java
deleted file mode 100644
index 2f68135dee1..00000000000
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/beans/RestoreCollectionPayload.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.client.solrj.request.beans;
-
-import static org.apache.solr.client.solrj.request.beans.V2ApiConstants.CREATE_COLLECTION_KEY;
-
-import java.util.Map;
-import org.apache.solr.common.annotation.JsonProperty;
-import org.apache.solr.common.util.ReflectMapWriter;
-
-/**
- * V2 API POJO for the /v2/collections 'restore-collection' command.
- *
- * <p>Analogous to the request parameters for v1 /admin/collections?action=RESTORE API.
- */
-public class RestoreCollectionPayload implements ReflectMapWriter {
-
-  @JsonProperty(required = true)
-  public String collection;
-
-  @JsonProperty public String location;
-
-  @JsonProperty public String repository;
-
-  @JsonProperty public Integer backupId;
-
-  @JsonProperty(CREATE_COLLECTION_KEY)
-  public Map<String, Object> createCollectionParams;
-
-  @JsonProperty public String async;
-}