You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/10/25 18:01:50 UTC
[GitHub] [solr] Jdurham2843 opened a new pull request, #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Jdurham2843 opened a new pull request, #1126:
URL: https://github.com/apache/solr/pull/1126
https://issues.apache.org/jira/browse/SOLR-16462
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Solr:
* https://issues.apache.org/jira/projects/SOLR
For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA.
To create a Jira issue, you will need to create an account there first.
The title of the PR should reference the Jira issue number in the form:
* SOLR-####: <short description of problem or changes>
SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
This pull request implements V2 equivalent endpoints for the Snapshot API (Create, List, and Delete).
# Solution
I created a SnapshotAPI class that houses new v2 implementations of the CreateSnapshot, ListSnapshot, and DeleteSnapshot operations. The SnapshotAPI endpoints were implemented using the Jersey JAX-RS reference implementation. The original CoreAdminOperation implementations were modified to call the new SnapshotAPI methods. The backup/restore reference documentation has been updated to show both V1 and V2 Snapshot API examples.
# Tests
No new tests were added, but I would be willing to add some if needed. There are existing tests that check that both the coreName and commitName were passed into a given Snapshot operation. Since those values are now part of the URL, I didn't see any reason to test for that against the new V2 methods.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [x] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1346800289
That doesn't sound like any fun, but I totally get it and no worries! This work should be here whenever your in a position to look at it. Hopefully the regression isn't too hard to remediate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1293963646
I believe I've covered most of your first round of comments @gerlowskija , aside from tests and the uses of the squash method on the ListSnapshotAPI. I also responded to your thoughts on the async handling code back on the jira ticket as well. I'll work to add some tests focused on this new CoreSnapshotAPI class while we work through the our async game plan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "Jdurham2843 (via GitHub)" <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1435794483
From what I can tell, the injection exception isn't making it into any logs. Instead its being spat out into the terminal where I ran the solr start command. I'm only seeing this error when running in Windows (Powershell or Command Prompt). If I run it under Ubuntu (wsl2), I don't see the error at all. Maybe something OS specific?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "Jdurham2843 (via GitHub)" <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1438766806
Sounds good, thanks for working with me on this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1014259104
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +276,28 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final SnapshotAPI.ListSnapshotsResponse response = snapshotAPI.listSnapshots(coreName);
- SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
- @SuppressWarnings({"rawtypes"})
- NamedList result = new NamedList();
- for (String name : mgr.listSnapshots()) {
- Optional<SnapshotMetaData> metadata = mgr.getSnapshotMetaData(name);
- if (metadata.isPresent()) {
- NamedList<String> props = new NamedList<>();
- props.add(
- SolrSnapshotManager.GENERATION_NUM,
- String.valueOf(metadata.get().getGenerationNumber()));
- props.add(SolrSnapshotManager.INDEX_DIR_PATH, metadata.get().getIndexDirPath());
- result.add(name, props);
- }
- }
- it.rsp.add(SolrSnapshotManager.SNAPSHOTS_INFO, result);
+ final NamedList<Object> snapshotsResult = new NamedList<>();
Review Comment:
After giving this a bit more thought, it'd be a real shame to cement this behavior, which is probably a bug in any case. If it's an unintentional bug, then it doesn't fall under Solr's standard backcompat policy: we're allowed to fix bugs after all.
I started a discussion about this on the 'dev@' mailing list. Hopefully someone else will chime in there with agreement, at which point I'm happy to go forward with the corrected syntax your PR uses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1438622911
> I'm only seeing this error when running in Windows
Huh, that's a weird wrinkle. There's nothing OS-specific (that I'm aware of) in any of the dependency-injection stuff. And, fwiw, I took another stab at reproducing on MacOS this morning and couldn't see any of the logs either.
If I had to guess, I'd say that this is probably related to how Solr uses separate Jersey applications for "container" and "per-core" APIs. There are some situations where Solr isn't sure what "type" an incoming request is, so it tries sending the request to one Jersey app, and only tries the second if the first errors out. That'd help explain how we see clear errors in the logs despite the request ultimately succeeding: one Jersey app fails with the observed errors before the second Jersey app succeeds.
But I'm stumped why this behavior would be OS dependent, or even why it'd fail with an injection error. In the scenario above the first Jersey app typically fails with a NotFoundException that is handled specially so that it doesn't pollute the logs.
----
In terms of moving forward here, if the requests failed on Windows it'd be a very different matter. But I think the extra noise in the logs is probably tolerable for now given that v2 is an experimental feature. Absolutely something that needs fixed, but it feels like a framework problem and not something inherent to this PR.
I'll plan on merging this to 'main' and letting it bake a bit before backporting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1007209747
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +276,28 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final SnapshotAPI.ListSnapshotsResponse response = snapshotAPI.listSnapshots(coreName);
- SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
- @SuppressWarnings({"rawtypes"})
- NamedList result = new NamedList();
- for (String name : mgr.listSnapshots()) {
- Optional<SnapshotMetaData> metadata = mgr.getSnapshotMetaData(name);
- if (metadata.isPresent()) {
- NamedList<String> props = new NamedList<>();
- props.add(
- SolrSnapshotManager.GENERATION_NUM,
- String.valueOf(metadata.get().getGenerationNumber()));
- props.add(SolrSnapshotManager.INDEX_DIR_PATH, metadata.get().getIndexDirPath());
- result.add(name, props);
- }
- }
- it.rsp.add(SolrSnapshotManager.SNAPSHOTS_INFO, result);
+ final NamedList<Object> snapshotsResult = new NamedList<>();
Review Comment:
I hadn't seen that method before, but that seems like the better approach! I refactored all three operations to use it with success on all but the v1 ListSnapshots operation. From what I've seen, the original method returns a response with the following structure:
```
{
"responseHeader": {
"status": 0,
"QTime": 0
},
"snapshots": [
"snapshot4",
[
"generation",
"2",
"indexDirPath",
"C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
],
"snapshot3",
[
"generation",
"2",
"indexDirPath",
"C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
],
"snapshot2",
[
"generation",
"2",
"indexDirPath",
"C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
],
"snapshot6",
[
"generation",
"2",
"indexDirPath",
"C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
]
]
}
```
While using the squash method returns the following structure:
```
{
"responseHeader": {
"status": 0,
"QTime": 7
},
"snapshots": {
"snapshot4": {
"generation": 2,
"indexDirPath": "C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
},
"snapshot3": {
"generation": 2,
"indexDirPath": "C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
},
"snapshot2": {
"generation": 2,
"indexDirPath": "C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
},
"snapshot6": {
"generation": 2,
"indexDirPath": "C:\\<redacted-path>\\solr\\solr\\packaging\\build\\dev\\example\\techproducts\\solr\\techproducts\\data\\index/"
}
}
}
```
Would the change in the structure of the response for the V1 endpoint be considered a breaking change that we should prevent?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1107644285
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1095,26 +1095,21 @@ protected void configure() {
.to(SolrNodeKeyPair.class)
.in(Singleton.class);
}
+ })
+ .register(
+ new AbstractBinder() {
+ @Override
+ protected void configure() {
+ bindFactory(
+ new InjectionFactories.SingletonFactory<>(
+ coreAdminHandler.getCoreAdminAsyncTracker()))
+ .to(CoreAdminHandler.CoreAdminAsyncTracker.class)
+ .in(Singleton.class);
+ }
});
jerseyAppHandler = new ApplicationHandler(containerHandlers.getJerseyEndpoints());
}
- // Do Node setup logic after all handlers have been registered.
Review Comment:
[Q] Hmm...Do you know why this is deleted on your branch? Doesn't seem related to this JIRA...
##########
solr/core/src/java/org/apache/solr/handler/admin/api/CoreSnapshotAPI.java:
##########
@@ -0,0 +1,280 @@
+/*
+ * 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.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_READ_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.handler.admin.CoreAdminHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/** V2 API for Creating, Listing, and Deleting Core Snapshots. */
+@Path("/cores/{coreName}/snapshots")
+public class CoreSnapshotAPI extends CoreAdminAPIBase {
+
+ @Inject
+ public CoreSnapshotAPI(
+ SolrQueryRequest request,
+ SolrQueryResponse response,
+ CoreContainer coreContainer,
+ CoreAdminHandler.CoreAdminAsyncTracker coreAdminAsyncTracker) {
+ super(coreContainer, coreAdminAsyncTracker, request, response);
+ }
+
+ /** This API is analogous to V1 (POST /solr/admin/cores?action=CREATESNAPSHOT) */
+ @POST
+ @Path("/{snapshotName}")
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(CORE_EDIT_PERM)
+ public CreateSnapshotResponse createSnapshot(
+ @Parameter(description = "The name of the core to snapshot.", required = true)
+ @PathParam("coreName")
+ String coreName,
+ @Parameter(description = "The name to associate with the core snapshot.", required = true)
+ @PathParam("snapshotName")
+ String snapshotName,
+ @Parameter(description = "The id to associate with the async task.") @QueryParam("async")
+ String taskId)
+ throws Exception {
+ final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+
+ return handle(
+ response,
+ coreName,
+ taskId,
+ "createSnapshot",
+ () -> {
+ try (SolrCore core = coreContainer.getCore(coreName)) {
+ if (core == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + coreName);
+ }
+
+ final String indexDirPath = core.getIndexDir();
+ final IndexDeletionPolicyWrapper delPol = core.getDeletionPolicy();
+ final IndexCommit ic = delPol.getAndSaveLatestCommit();
+ try {
+ if (null == ic) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "No index commits to snapshot in core " + coreName);
+ }
+ final SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
+ mgr.snapshot(snapshotName, indexDirPath, ic.getGeneration());
+
+ response.core = core.getName();
+ response.commitName = snapshotName;
+ response.indexDirPath = indexDirPath;
+ response.generation = ic.getGeneration();
+ response.files = ic.getFileNames();
+ } catch (IOException e) {
+ throw new CoreAdminAPIBaseException(e);
+ } finally {
+ delPol.releaseCommitPoint(ic);
+ }
+ }
+
+ return response;
+ });
+ }
+
+ /** The Response for {@link CoreSnapshotAPI}'s {@link #createSnapshot(String, String, String)} */
+ public static class CreateSnapshotResponse extends SolrJerseyResponse {
+ @Schema(description = "The name of the core.")
Review Comment:
[+1] Awesome, thanks for the OpenAPI docs on these fields!
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java:
##########
@@ -493,4 +434,94 @@ public interface CoreAdminOp {
*/
void execute(CallInfo it) throws Exception;
}
+
+ public static class CoreAdminAsyncTracker {
+ private static final int MAX_TRACKED_REQUESTS = 100;
+ public static final String RUNNING = "running";
+ public static final String COMPLETED = "completed";
+ public static final String FAILED = "failed";
+ public final Map<String, Map<String, TaskObject>> requestStatusMap;
+
+ private ExecutorService parallelExecutor =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ 50, new SolrNamedThreadFactory("parallelCoreAdminAPIBaseExecutor"));
+
+ public CoreAdminAsyncTracker() {
+ HashMap<String, Map<String, TaskObject>> map = new HashMap<>(3, 1.0f);
+ map.put(RUNNING, Collections.synchronizedMap(new LinkedHashMap<>()));
+ map.put(COMPLETED, Collections.synchronizedMap(new LinkedHashMap<>()));
+ map.put(FAILED, Collections.synchronizedMap(new LinkedHashMap<>()));
+ requestStatusMap = Collections.unmodifiableMap(map);
+ }
+
+ public void shutdown() {
+ ExecutorUtil.shutdownAndAwaitTermination(parallelExecutor);
+ }
+
+ public Map<String, TaskObject> getRequestStatusMap(String key) {
+ return requestStatusMap.get(key);
+ }
+
+ public ExecutorService getParallelExecutor() {
+ return parallelExecutor;
+ }
+
+ /** Helper method to add a task to a tracking type. */
+ public void addTask(String type, TaskObject o, boolean limit) {
+ synchronized (getRequestStatusMap(type)) {
+ if (limit && getRequestStatusMap(type).size() == MAX_TRACKED_REQUESTS) {
+ String key = getRequestStatusMap(type).entrySet().iterator().next().getKey();
+ getRequestStatusMap(type).remove(key);
+ }
+ addTask(type, o);
+ }
+ }
+
+ public void addTask(String type, TaskObject o) {
+ synchronized (getRequestStatusMap(type)) {
+ getRequestStatusMap(type).put(o.taskId, o);
+ }
+ }
+
+ /** Helper method to remove a task from a tracking map. */
+ public void removeTask(String map, String taskId) {
+ synchronized (getRequestStatusMap(map)) {
Review Comment:
Let's ignore this 'lift' warning for now - it's coming from code that has only been moved around (and not changed) as a part of this PR.
Might be worth adding a TODO comment somewhere in this code though that proposes making the status options (RUNNING, COMPLETED, FAILED) into an enum though, to provide a little more type-safety going forward.
##########
solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc:
##########
@@ -308,37 +330,45 @@ The name of the core to whose snapshots we want to list.
+
Request ID to track this action which will be processed asynchronously.
-=== Delete Snapshot API
+=== LIST Response
+
+The response will include the status of the request and all existing snapshots for the core.
+If the status is anything other than "success", an error message will explain why the request failed.
-The `DELETESNAPSHOT` command deletes a snapshot for a particular core.
+[[delete-snapshot-api]]
+== DELETE: Delete a Snapshot
You can trigger a delete snapshot with an HTTP command like this (replace "techproducts" with the name of the core you are working with):
-.Delete Snapshot API Example
-[source,text]
+[.dynamic-tabs]
+--
+[example.tab-pane#v1deletesnapshot]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
----
-http://localhost:8983/solr/admin/cores?action=DELETESNAPSHOT&core=techproducts&commitName=commit1
+curl -X DELETE http://localhost:8983/solr/admin/cores?action=DELETESNAPSHOT&core=techproducts&commitName=commit1
Review Comment:
[-0] We should probably drop the `-X DELETE` in the v1 example, it's never been documented that way before and I'd be surprised if anything other than a GET or POST worked for v1.
##########
solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc:
##########
@@ -276,28 +281,45 @@ The name of the core to perform the snapshot on.
+
Request ID to track this action which will be processed asynchronously.
-=== List Snapshot API
+=== CREATE Response
+
+The response will include the status of the request, the core name, snapshot name, index directory path, snapshot generation, and a list of file names.
+If the status is anything other than "success", an error message will explain why the request failed.
-The `LISTSNAPSHOTS` command lists all the taken snapshots for a particular core.
+[[list-snapshot-api]]
+== List: List All Snapshots for a Particular Core
You can trigger a list snapshot command with an HTTP command like this (replace "techproducts" with the name of the core you are working with):
-.List Snapshot API
-[source,text]
+[.dynamic-tabs]
+--
+[example.tab-pane#v1listsnapshots]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
----
-http://localhost:8983/solr/admin/cores?action=LISTSNAPSHOTS&core=techproducts&commitName=commit1
+curl http://localhost:8983/solr/admin/cores?action=LISTSNAPSHOTS&core=techproducts&commitName=commit1
+
----
+====
-The list snapshot request parameters are:
+[example.tab-pane#v2listsnapshots]
+====
+[.tab-label]*V2 API*
-`core`::
-+
-[%autowidth,frame=none]
-|===
-|Optional |Default: none
-|===
-+
-The name of the core to whose snapshots we want to list.
+With the v2 API, the core and snapshot names are part of the path instead of query parameters.
Review Comment:
[-0] Small nit: this looks like a copy/paste oversight - there is no snapshot name in the LIST API.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * 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.handler.admin.CoreAdminHandler.CoreAdminAsyncTracker.COMPLETED;
+import static org.apache.solr.handler.admin.CoreAdminHandler.CoreAdminAsyncTracker.FAILED;
+import static org.apache.solr.handler.admin.CoreAdminHandler.CoreAdminAsyncTracker.RUNNING;
+
+import java.util.function.Supplier;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CoreAdminHandler;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.MDC;
+
+/**
+ * A common parent for admin Core Jersey-based APIs.
+ *
+ * <p>This base class is used when creating Core APIs to allow extra bookkeeping tasks such as async
+ * requests handling.
+ */
+public abstract class CoreAdminAPIBase extends JerseyResource {
+
+ protected final CoreContainer coreContainer;
+ protected final CoreAdminHandler.CoreAdminAsyncTracker coreAdminAsyncTracker;
+ protected final SolrQueryRequest req;
+ protected final SolrQueryResponse rsp;
+
+ public CoreAdminAPIBase(
+ CoreContainer coreContainer,
+ CoreAdminHandler.CoreAdminAsyncTracker coreAdminAsyncTracker,
+ SolrQueryRequest req,
+ SolrQueryResponse rsp) {
+ this.coreContainer = coreContainer;
+ this.coreAdminAsyncTracker = coreAdminAsyncTracker;
+ this.req = req;
+ this.rsp = rsp;
+ }
+
+ /**
+ * Wraps the subclasses logic with extra bookkeeping logic.
+ *
+ * <p>This method currently exists to enable async handling behavior for V2 Core APIs.
+ *
+ * <p>Since the logic for a given API lives inside the Supplier functional interface, checked
+ * exceptions can't be thrown directly to the calling method. To throw a checked exception out of
+ * the Supplier, wrap the exception using {@link CoreAdminAPIBase.CoreAdminAPIBaseException} and
+ * throw it instead. This handle method will retrieve the checked exception from {@link
+ * CoreAdminAPIBaseException} and throw it to the original calling method.
+ *
+ * @param solrJerseyResponse the response that the calling methods expects to return.
+ * @param coreName the name of the core that work is being done against.
+ * @param taskId an id provided for registering async work.
+ * @param actionName a name for the action being done.
+ * @param supplier the work that the calling method wants done.
+ * @return the supplied T solrJerseyResponse
+ */
+ public <T extends SolrJerseyResponse> T handle(
Review Comment:
[+1] Overall I really like how this approach panned out; very cool!
[0] Small nit: can you think of a name here that'd make the method's purpose clearer than `handle`? I'm struggling a bit, but 'handle' feels generic. Maybe `handlePotentiallyAsynchronousTask`?
[-0] The one downside of this approach is that CoreAdminHandler still has a lot of code that looks similar to what's in this method. (Specifically lines 200-250ish in CoreAdminHandler.) Maybe this is as good as it gets, but can you think of any way we could refactor CoreAdminHandler, or this method, or both to cut down on the duplication between the two?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "Jdurham2843 (via GitHub)" <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1433915434
Hey! I'll start testing the changes you made, and see if the injection errors are still around. From a first look at the changes, I agree with all of them. Regarding the near-duplicate async processing code: I remember trying to dedupe it, but hit a point where I couldn't see how to do it without creating a mess. I'm glad you found a way to make it work, because it felt kind of wrong to leave that in the codebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "sonatype-lift[bot] (via GitHub)" <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1108778401
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java:
##########
@@ -493,4 +404,133 @@ public interface CoreAdminOp {
*/
void execute(CallInfo it) throws Exception;
}
+
+ public static class CoreAdminAsyncTracker {
+ private static final int MAX_TRACKED_REQUESTS = 100;
+ public static final String RUNNING = "running";
+ public static final String COMPLETED = "completed";
+ public static final String FAILED = "failed";
+ public final Map<String, Map<String, TaskObject>> requestStatusMap;
+
+ private ExecutorService parallelExecutor =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ 50, new SolrNamedThreadFactory("parallelCoreAdminAPIBaseExecutor"));
+
+ public CoreAdminAsyncTracker() {
+ HashMap<String, Map<String, TaskObject>> map = new HashMap<>(3, 1.0f);
+ map.put(RUNNING, Collections.synchronizedMap(new LinkedHashMap<>()));
+ map.put(COMPLETED, Collections.synchronizedMap(new LinkedHashMap<>()));
+ map.put(FAILED, Collections.synchronizedMap(new LinkedHashMap<>()));
+ requestStatusMap = Collections.unmodifiableMap(map);
+ }
+
+ public void shutdown() {
+ ExecutorUtil.shutdownAndAwaitTermination(parallelExecutor);
+ }
+
+ public Map<String, TaskObject> getRequestStatusMap(String key) {
+ return requestStatusMap.get(key);
+ }
+
+ public void submitAsyncTask(
+ TaskObject taskObject, String action, Callable<SolrQueryResponse> task)
+ throws SolrException {
+ ensureTaskIdNotInUse(taskObject.taskId);
+ addTask(RUNNING, taskObject);
+
+ try {
+ MDC.put("CoreAdminHandler.asyncId", taskObject.taskId);
+ MDC.put("CoreAdminHandler.action", action);
+ parallelExecutor.execute(
+ () -> {
+ boolean exceptionCaught = false;
+ try {
+ final SolrQueryResponse response = task.call();
+ taskObject.setRspObject(response);
+ taskObject.setOperationRspObject(response);
+ } catch (Exception e) {
+ exceptionCaught = true;
+ taskObject.setRspObjectFromException(e);
+ } finally {
+ finishTask(taskObject, !exceptionCaught);
+ }
+ });
+ } finally {
+ MDC.remove("CoreAdminHandler.asyncId");
+ MDC.remove("CoreAdminHandler.action");
+ }
+ }
+
+ /** Helper method to add a task to a tracking type. */
+ private void addTask(String type, TaskObject o, boolean limit) {
+ synchronized (getRequestStatusMap(type)) {
+ if (limit && getRequestStatusMap(type).size() == MAX_TRACKED_REQUESTS) {
+ String key = getRequestStatusMap(type).entrySet().iterator().next().getKey();
+ getRequestStatusMap(type).remove(key);
+ }
+ addTask(type, o);
+ }
+ }
+
+ private void addTask(String type, TaskObject o) {
+ synchronized (getRequestStatusMap(type)) {
+ getRequestStatusMap(type).put(o.taskId, o);
+ }
+ }
+
+ /** Helper method to remove a task from a tracking map. */
+ private void removeTask(String map, String taskId) {
+ synchronized (getRequestStatusMap(map)) {
+ getRequestStatusMap(map).remove(taskId);
+ }
+ }
+
+ private void ensureTaskIdNotInUse(String taskId) throws SolrException {
+ if (getRequestStatusMap(RUNNING).containsKey(taskId)
Review Comment:
<picture><img alt="5% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/5/display.svg"></picture>
<b>*NULLPTR_DEREFERENCE:</b>* `Map CoreAdminHandler$CoreAdminAsyncTracker.getRequestStatusMap(String)` could be null (from the call to `CoreAdminHandler$CoreAdminAsyncTracker.getRequestStatusMap(...)` on line 489) and is dereferenced.
---
<details><summary>âšī¸ Expand to see all <b>@sonatype-lift</b> commands</summary>
You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
| **Command** | **Usage** |
| ------------- | ------------- |
| `@sonatype-lift ignore` | Leave out the above finding from this PR |
| `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
| `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
**Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
<sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
---
<b>Help us improve LIFT! (<i>Sonatype LiftBot external survey</i>)</b>
Was this a good recommendation for you? <sub><small>Answering this survey will not impact your Lift settings.</small></sub>
[â[đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=394165237&lift_comment_rating=1)â] - [â[đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=394165237&lift_comment_rating=2)â] - [â[đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=394165237&lift_comment_rating=3)â] - [â[đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=394165237&lift_comment_rating=4)â] - [â[đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=394165237&lift_comment_rating=5)â]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1435446765
Hmm, I haven't noticed that Injection error in the logs, though maybe I missed it since the API returns successfully?
I'll play around with it some more and pay more attention to the logs on successful API calls. đ¤
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "Jdurham2843 (via GitHub)" <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1433972124
I did some testing and things appeared to be working correctly. I am still seeing the injection exception. Any thoughts what that might be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "Jdurham2843 (via GitHub)" <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1427188596
Yep, I think it should be ready for review! I made an attempt to handle async requests, but if you decide there's a better way to do it, I can look at reverting those changes. I also went ahead and did a rebase to bring the branch up to date with main as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1007229294
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SnapshotAPI.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_READ_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+@Path("/cores/{coreName}/snapshots")
+public class SnapshotAPI extends JerseyResource {
+
+ private final CoreContainer coreContainer;
+
+ @Inject
+ public SnapshotAPI(CoreContainer coreContainer) {
+ this.coreContainer = coreContainer;
+ }
+
+ @POST
+ @Path("/{snapshotName}")
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(CORE_EDIT_PERM)
+ public CreateSnapshotResponse createSnapshot(
+ @Parameter(description = "The name of the core to snapshot.", required = true)
+ @PathParam("coreName")
+ String coreName,
+ @Parameter(description = "The name to associate with the core snapshot.", required = true)
+ @PathParam("snapshotName")
+ String snapshotName)
+ throws Exception {
+ final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+
+ try (SolrCore core = coreContainer.getCore(coreName)) {
+ if (core == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + coreName);
+ }
+
+ final String indexDirPath = core.getIndexDir();
+ final IndexDeletionPolicyWrapper delPol = core.getDeletionPolicy();
+ final IndexCommit ic = delPol.getAndSaveLatestCommit();
+ try {
+ if (null == ic) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "No index commits to snapshot in core " + coreName);
+ }
+ final SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
+ mgr.snapshot(snapshotName, indexDirPath, ic.getGeneration());
+
+ response.core = core.getName();
+ response.commitName = snapshotName;
+ response.indexDirPath = indexDirPath;
+ response.generation = ic.getGeneration();
+ response.files = ic.getFileNames();
+ } finally {
+ delPol.releaseCommitPoint(ic);
+ }
+ }
+
+ return response;
+ }
+
+ public static class CreateSnapshotResponse extends SolrJerseyResponse {
+ @JsonProperty(CoreAdminParams.CORE)
Review Comment:
I'll make an effort to add some Schema statements. It seems like a good idea to add those now while we're working on this API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1020964514
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +282,16 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final CoreSnapshotAPI coreSnapshotAPI =
+ new CoreSnapshotAPI(it.req, it.rsp, coreContainer, null);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final CoreSnapshotAPI.ListSnapshotsResponse response =
+ coreSnapshotAPI.listSnapshots(coreName, null);
Review Comment:
đŦ 2 similar findings have been found in this PR
---
*NULL_DEREFERENCE:* object `coreSnapshotAPI.coreAdminAsyncTracker` last assigned on line 288 could be null and is dereferenced by call to `listSnapshots(...)` at line 292.
---
<details><summary><b>đ Expand here to view all instances of this finding</b></summary><br/>
<div align="center">
| **File Path** | **Line Number** |
| ------------- | ------------- |
| solr/core/src/java/org/apache/solr/handler/admin/CreateSnapshotOp.java | [38](https://github.com/Jdurham2843/solr/blob/1b14c72db477f9f66aac04313e304dcc1797b0e7/solr/core/src/java/org/apache/solr/handler/admin/CreateSnapshotOp.java#L38)|
| solr/core/src/java/org/apache/solr/handler/admin/DeleteSnapshotOp.java | [39](https://github.com/Jdurham2843/solr/blob/1b14c72db477f9f66aac04313e304dcc1797b0e7/solr/core/src/java/org/apache/solr/handler/admin/DeleteSnapshotOp.java#L39)|
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GHSAW71J7A3ZF3T03364RD45?t=Infer|NULL_DEREFERENCE" target="_blank">Visit the Lift Web Console</a> to find more details in your report.</p></div></details>
---
<details><summary><b>âšī¸ Learn about @sonatype-lift commands</b></summary>
You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
| **Command** | **Usage** |
| ------------- | ------------- |
| `@sonatype-lift ignore` | Leave out the above finding from this PR |
| `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
| `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
**Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
<sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
---
Was this a good recommendation?
[ [đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=352787933&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=352787933&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=352787933&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=352787933&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=352787933&lift_comment_rating=5) ]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1007210960
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SnapshotAPI.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_READ_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+@Path("/cores/{coreName}/snapshots")
+public class SnapshotAPI extends JerseyResource {
Review Comment:
That's a good idea - I'll change the name!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1433337997
Morning @Jdurham2843 - I took the liberty of addressing my review comments as they were all relatively minor. I handled each one as a separate commit, so if you happen to disagree on any it should be pretty easy to just revert that one without undoing the others.
The largest of these changes was the elimination of the near-duplicate async-processing code from CoreAdminAPIBase.handle and CoreAdminHandler. This was a little tricky but I ended up managing it by having CoreAdminAsyncTracker provide a slightly higher level of abstraction and moving a lot of the duplicated bits (MDC handling, map insertion/removal) internal to that class. Let me know what you think about that approach when you get a chance to look.
----
With my review comments cleared away I've begun testing locally with an eye to commit this soon, so let me know if you have any reservations or anything else you were still hoping to do on this PR!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1020336101
##########
solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.MDC;
+
+public abstract class CoreAdminAPIBase extends JerseyResource {
Review Comment:
That sounds like a good idea, I'll see about adding those!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1303884400
Recent changes (`@Schema` documentation, failure-case unit tests, API class renaming) all look great.
To summarize where we are, I think there's two remaining items to clear up before this can be merged:
1. A decision on whether we should hold strict backcompat for the v1 LISTSNAPSHOT response format, or go with the (clearly preferable) syntax the v2 API has currently. I opened a dev@ thread about that, which slows this PR down a bit, but it's better to get consensus on this rather than risk someone complaining later. If there's no objections by mid-next week I say we go forward with the improved syntax.
2. Handling of the async param. I think I offered to help with this in JIRA; so I'll take a crack at it if you haven't already started in on it @Jdurham2843 ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1009599949
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +276,28 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final SnapshotAPI.ListSnapshotsResponse response = snapshotAPI.listSnapshots(coreName);
- SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
- @SuppressWarnings({"rawtypes"})
- NamedList result = new NamedList();
- for (String name : mgr.listSnapshots()) {
- Optional<SnapshotMetaData> metadata = mgr.getSnapshotMetaData(name);
- if (metadata.isPresent()) {
- NamedList<String> props = new NamedList<>();
- props.add(
- SolrSnapshotManager.GENERATION_NUM,
- String.valueOf(metadata.get().getGenerationNumber()));
- props.add(SolrSnapshotManager.INDEX_DIR_PATH, metadata.get().getIndexDirPath());
- result.add(name, props);
- }
- }
- it.rsp.add(SolrSnapshotManager.SNAPSHOTS_INFO, result);
+ final NamedList<Object> snapshotsResult = new NamedList<>();
Review Comment:
Oof. I really don't want to defend that original v1 format, which looks clearly wrong to me. But I think you're right to suspect that changing this would be a "breaking change" that we should prevent đ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1297329730
Awesome; thanks John. I replied on JIRA, about the async stuff as well, so I think that's everything. Lmk if I missed a question though!
Feel free to push up whatever you've got from time to time if you're interested in a sanity check on the "delta", even if it's not quite "finished" yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1016898284
##########
solr/core/src/test/org/apache/solr/handler/admin/api/CoreSnapshotAPITest.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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 java.util.ArrayList;
+import java.util.List;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class CoreSnapshotAPITest extends SolrTestCaseJ4 {
Review Comment:
[+1] These tests look awesome! Nice to avoid heavy mocking and also avoid standing up a full SolrCloud cluster or anything similarly-heavy like that.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.MDC;
+
+public abstract class CoreAdminAPIBase extends JerseyResource {
Review Comment:
[0] Small nit, should add Javadocs explaining what this base class handles for callers and when/how it should be used.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.MDC;
+
+public abstract class CoreAdminAPIBase extends JerseyResource {
+
+ private static final ExecutorService PARALLEL_EXECUTOR =
Review Comment:
[-1] I love the approach on this class overall (esp. the `handle` method that takes in a Supplier), but I'm concerned about reliance on static state here.
I hinted at this a bit on JIRA, but should've been more explicit:
> I guess we could create a base-class for our APIs, e.g. "AsyncTrackingApiBase", with a static instance to hold this status info, that gets extended by various API subclasses. I don't love the idea of introducing non-constant static state, but I think it'd work?
It's my fault for not being more explicit: it's not the norm, but it's also not unheard of to run multiple Solr instances from within the same JVM. Solr's tests do it, for one, and I've heard of actual applications doing this too on occasion via EmbeddedSolrServer and similar classes.
Maybe this would "just work" if the tests and EmbeddedSolrServer know to use different classloaders, but if it doesn't, then the use of static state could cause tricky bugs - e.g. the same executor being used for multiple Solr instances.
I'll run the test suite and poke around some of the related code to double-check my fears here, but assuming they're well founded, we'll need a different approach here.
One approach that's only slightly a "dodge" would be to essentially re-use the same status-map and `parallelExecutor` that CoreAdminHandler creates. (i.e. Add getters for these things to `CoreAdminHandler` and then modify the Jersey application to make the status-map and executor injectable as singletons, similar to what we do for CoreContainer [here](https://github.com/apache/solr/blob/c86128855807042e25c5bd320e6f7729784e8884/solr/core/src/java/org/apache/solr/core/CoreContainer.java#L1067))
This would have a few side-benefits:
- avoids the creation of an additional executor (the current PR creates one in the handler and one in this API class)
- no executor lifecycle changes required, since we're reusing the executor
- injection gives the API classes enough indirection that they won't require any changes when we eventually nuke v1 and need to find a different way of managing the executor/status-map
- no duplication of constants and status-map init logic (current PR has this code in both the handler and this API class).
Any thoughts on that? I'm happy to make the changes required as the rework is largely my fault for not communicating my concerns around static-state strongly enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1020979179
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +282,16 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final CoreSnapshotAPI coreSnapshotAPI =
+ new CoreSnapshotAPI(it.req, it.rsp, coreContainer, null);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final CoreSnapshotAPI.ListSnapshotsResponse response =
+ coreSnapshotAPI.listSnapshots(coreName, null);
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1312846199
Update:
1. I added some JavaDocs to CoreAdminAPIBase and CoreSnapshotAPI. I tried to follow the same general patterns I saw in JerseyResource and in the SchemaNameAPI class.
2. I made an attempt at reusing the executor and statusMap from CoreAdminHandler in the CoreAdminAPIBase class. Instead of Injecting both separately, I moved them both into a static nested class on CoreAdminHandler and wrote some code to Inject that. Even if we decide not to use this version and instead you have a better approach, we can revert these changes. I at least wanted to give it a shot first!
3. I added the V2 squash static method to the V1 ListSnapshots operation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1005968009
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +276,28 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final SnapshotAPI.ListSnapshotsResponse response = snapshotAPI.listSnapshots(coreName);
- SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
- @SuppressWarnings({"rawtypes"})
- NamedList result = new NamedList();
- for (String name : mgr.listSnapshots()) {
- Optional<SnapshotMetaData> metadata = mgr.getSnapshotMetaData(name);
- if (metadata.isPresent()) {
- NamedList<String> props = new NamedList<>();
- props.add(
- SolrSnapshotManager.GENERATION_NUM,
- String.valueOf(metadata.get().getGenerationNumber()));
- props.add(SolrSnapshotManager.INDEX_DIR_PATH, metadata.get().getIndexDirPath());
- result.add(name, props);
- }
- }
- it.rsp.add(SolrSnapshotManager.SNAPSHOTS_INFO, result);
+ final NamedList<Object> snapshotsResult = new NamedList<>();
Review Comment:
[Q] With some of our other JAX-RS based APIs, we're able to implement the v1 code by taking the strongly-typed response and calling `V2ApiUtils.squashIntoSolrResponseWIthoutHeader`.
Elsewhere that's allowed us to simplify the v1 code - we don't have to copy individual fields over to the NamedList, etc.
Do you think that's something that would work here, or did you have a reason to intentionally avoid that approach here? At a glance, it seems like it'd work, but there might be something I'm missing.
(You can find an example of this "squashing" [here](https://github.com/apache/solr/blob/3e4e1e681a572a1b9b1f56a03ca459e1dc1544d4/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java#L1298), for reference.)
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SnapshotAPI.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_READ_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+@Path("/cores/{coreName}/snapshots")
+public class SnapshotAPI extends JerseyResource {
+
+ private final CoreContainer coreContainer;
+
+ @Inject
+ public SnapshotAPI(CoreContainer coreContainer) {
+ this.coreContainer = coreContainer;
+ }
+
+ @POST
+ @Path("/{snapshotName}")
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(CORE_EDIT_PERM)
+ public CreateSnapshotResponse createSnapshot(
+ @Parameter(description = "The name of the core to snapshot.", required = true)
+ @PathParam("coreName")
+ String coreName,
+ @Parameter(description = "The name to associate with the core snapshot.", required = true)
+ @PathParam("snapshotName")
+ String snapshotName)
+ throws Exception {
+ final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+
+ try (SolrCore core = coreContainer.getCore(coreName)) {
+ if (core == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + coreName);
+ }
+
+ final String indexDirPath = core.getIndexDir();
+ final IndexDeletionPolicyWrapper delPol = core.getDeletionPolicy();
+ final IndexCommit ic = delPol.getAndSaveLatestCommit();
+ try {
+ if (null == ic) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "No index commits to snapshot in core " + coreName);
+ }
+ final SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
+ mgr.snapshot(snapshotName, indexDirPath, ic.getGeneration());
+
+ response.core = core.getName();
+ response.commitName = snapshotName;
+ response.indexDirPath = indexDirPath;
+ response.generation = ic.getGeneration();
+ response.files = ic.getFileNames();
+ } finally {
+ delPol.releaseCommitPoint(ic);
+ }
+ }
+
+ return response;
+ }
+
+ public static class CreateSnapshotResponse extends SolrJerseyResponse {
+ @JsonProperty(CoreAdminParams.CORE)
Review Comment:
[0] It's not required, especially in cases where the nuances of what a particular field represents are unclear, but if you DO have a good idea for any of these fields and are willing, you can use the `@Schema` annotation to document it.
These descriptions end up in the OpenAPI spec generated by the Solr build, which we'll use in subsequent releases to create Solr clients for different languages (e.g. golang, Python). We're also hoping to use them to auto-generate certain sections of our ref-guide. So lots of indirect value there if you're able to document any of these.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SnapshotAPI.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_READ_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+@Path("/cores/{coreName}/snapshots")
+public class SnapshotAPI extends JerseyResource {
Review Comment:
[-1] Since we have snapshot APIs at both the "core" and "collection" levels, we should probably name this class in a way that makes it easy to distinguish which of the two it refers to. Maybe `CoreSnapshotAPI`?
##########
solr/core/src/java/org/apache/solr/handler/admin/api/SnapshotAPI.java:
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.client.solrj.impl.BinaryResponseParser.BINARY_CONTENT_TYPE_V2;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;
+import static org.apache.solr.security.PermissionNameProvider.Name.CORE_READ_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.lucene.index.IndexCommit;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.IndexDeletionPolicyWrapper;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
+import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+
+@Path("/cores/{coreName}/snapshots")
+public class SnapshotAPI extends JerseyResource {
+
+ private final CoreContainer coreContainer;
+
+ @Inject
+ public SnapshotAPI(CoreContainer coreContainer) {
+ this.coreContainer = coreContainer;
+ }
+
+ @POST
+ @Path("/{snapshotName}")
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(CORE_EDIT_PERM)
+ public CreateSnapshotResponse createSnapshot(
+ @Parameter(description = "The name of the core to snapshot.", required = true)
+ @PathParam("coreName")
+ String coreName,
+ @Parameter(description = "The name to associate with the core snapshot.", required = true)
+ @PathParam("snapshotName")
+ String snapshotName)
+ throws Exception {
+ final CreateSnapshotResponse response = instantiateJerseyResponse(CreateSnapshotResponse.class);
+
+ try (SolrCore core = coreContainer.getCore(coreName)) {
+ if (core == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + coreName);
+ }
+
+ final String indexDirPath = core.getIndexDir();
+ final IndexDeletionPolicyWrapper delPol = core.getDeletionPolicy();
+ final IndexCommit ic = delPol.getAndSaveLatestCommit();
+ try {
+ if (null == ic) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "No index commits to snapshot in core " + coreName);
+ }
+ final SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
+ mgr.snapshot(snapshotName, indexDirPath, ic.getGeneration());
+
+ response.core = core.getName();
+ response.commitName = snapshotName;
+ response.indexDirPath = indexDirPath;
+ response.generation = ic.getGeneration();
+ response.files = ic.getFileNames();
+ } finally {
+ delPol.releaseCommitPoint(ic);
+ }
+ }
+
+ return response;
+ }
+
+ public static class CreateSnapshotResponse extends SolrJerseyResponse {
+ @JsonProperty(CoreAdminParams.CORE)
+ public String core;
+
+ @JsonProperty(CoreAdminParams.SNAPSHOT_NAME)
+ public String commitName;
+
+ @JsonProperty(SolrSnapshotManager.INDEX_DIR_PATH)
+ public String indexDirPath;
+
+ @JsonProperty(SolrSnapshotManager.GENERATION_NUM)
+ public long generation;
+
+ @JsonProperty(SolrSnapshotManager.FILE_LIST)
+ public Collection<String> files;
+ }
+
+ @GET
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(CORE_READ_PERM)
+ public ListSnapshotsResponse listSnapshots(
+ @Parameter(
+ description = "The name of the core for which to retrieve snapshots.",
+ required = true)
+ @PathParam("coreName")
+ String coreName) {
+ final ListSnapshotsResponse response = instantiateJerseyResponse(ListSnapshotsResponse.class);
+
+ try (SolrCore core = coreContainer.getCore(coreName)) {
+ if (core == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + coreName);
+ }
+
+ SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
+
+ final Map<String, SnapshotInformation> result = new HashMap<>();
+ for (String name : mgr.listSnapshots()) {
+ Optional<SolrSnapshotMetaDataManager.SnapshotMetaData> metadata =
+ mgr.getSnapshotMetaData(name);
+ if (metadata.isPresent()) {
+ final SnapshotInformation snapshotInformation =
+ new SnapshotInformation(
+ metadata.get().getGenerationNumber(), metadata.get().getIndexDirPath());
+ result.put(name, snapshotInformation);
+ }
+ }
+
+ response.snapshots = result;
+ }
+
+ return response;
+ }
+
+ public static class ListSnapshotsResponse extends SolrJerseyResponse {
+ @JsonProperty(SolrSnapshotManager.SNAPSHOTS_INFO)
+ public Map<String, SnapshotInformation> snapshots;
+ }
+
+ public static class SnapshotInformation {
+ @JsonProperty(SolrSnapshotManager.GENERATION_NUM)
+ public final long generationNumber;
+
+ @JsonProperty(SolrSnapshotManager.INDEX_DIR_PATH)
+ public final String indexDirPath;
+
+ public SnapshotInformation(long generationNumber, String indexDirPath) {
+ this.generationNumber = generationNumber;
+ this.indexDirPath = indexDirPath;
+ }
+ }
+
+ @DELETE
+ @Path("/{snapshotName}")
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(CORE_EDIT_PERM)
+ public DeleteSnapshotResponse deleteSnapshot(
+ @Parameter(
+ description = "The name of the core for which to delete a snapshot.",
+ required = true)
+ @PathParam("coreName")
+ String coreName,
+ @Parameter(description = "The name of the core snapshot to delete.", required = true)
+ @PathParam("snapshotName")
+ String snapshotName)
+ throws Exception {
+ final DeleteSnapshotResponse response = instantiateJerseyResponse(DeleteSnapshotResponse.class);
+
+ final SolrCore core = coreContainer.getCore(coreName);
+ if (core == null) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + coreName);
+ }
+
+ try {
+ core.deleteNamedSnapshot(snapshotName);
+ // Ideally we shouldn't need this. This is added since the RPC logic in
Review Comment:
[0] I know this comment was there in the original code that you moved from DeleteSnapshotOp, so this isn't a comment on your PR, but it makes no sense to me.
What is the "this" being referred to? Adding the coreName to the response?
Anyway, just talking out loud I guess....
##########
solr/core/src/java/org/apache/solr/handler/admin/DeleteSnapshotOp.java:
##########
@@ -17,35 +17,26 @@
package org.apache.solr.handler.admin;
-import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.SolrCore;
+import org.apache.solr.handler.admin.api.SnapshotAPI;
class DeleteSnapshotOp implements CoreAdminHandler.CoreAdminOp {
@Override
public void execute(CoreAdminHandler.CallInfo it) throws Exception {
final SolrParams params = it.req.getParams();
- String commitName = params.required().get(CoreAdminParams.COMMIT_NAME);
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String commitName = params.required().get(CoreAdminParams.COMMIT_NAME);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
- SolrCore core = cc.getCore(cname);
- if (core == null) {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try {
- core.deleteNamedSnapshot(commitName);
- // Ideally we shouldn't need this. This is added since the RPC logic in
- // OverseerCollectionMessageHandler can not provide the coreName as part of the result.
- it.rsp.add(CoreAdminParams.CORE, core.getName());
- it.rsp.add(CoreAdminParams.COMMIT_NAME, commitName);
- } finally {
- core.close();
- }
+ final SnapshotAPI.DeleteSnapshotResponse response =
+ snapshotAPI.deleteSnapshot(coreName, commitName);
+
+ it.rsp.add(CoreAdminParams.CORE, response.coreName);
Review Comment:
[Q] ditto, re: can we use `V2ApiUtils.squashIntoSolrResponseWithoutHeader` here instead?
##########
solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc:
##########
@@ -234,38 +234,43 @@ http://localhost:8983/solr/gettingstarted/replication?command=restorestatus&wt=x
The status value can be "In Progress", "success" or "failed".
If it failed then an "exception" will also be sent in the response.
-=== Create Snapshot API
+[[create-snapshot-api]]
+== CREATE: Create a Snapshot
The snapshot functionality is different from the backup functionality as the index files aren't copied anywhere.
The index files are snapshotted in the same index directory and can be referenced while taking backups.
You can trigger a snapshot command with an HTTP command like this (replace "techproducts" with the name of the core you are working with):
-.Create Snapshot API Example
-[source,text]
+[.dynamic-tabs]
+--
+[example.tab-pane#v1createsnapshot]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
----
-http://localhost:8983/solr/admin/cores?action=CREATESNAPSHOT&core=techproducts&commitName=commit1
+curl -X POST http://localhost:8983/solr/admin/cores?action=CREATESNAPSHOT&core=techproducts&commitName=commit1
+
----
+====
-The `CREATESNAPSHOT` request parameters are:
+[example.tab-pane#v2createsnapshot]
+====
+[.tab-label]*V2 API*
-`commitName`::
-+
-[%autowidth,frame=none]
-|===
-|Optional |Default: none
-|===
-+
-The name to store the snapshot as.
+With the v2 API, the core and snapshot names are part of the path, no longer query parameters.
Review Comment:
[+1] Awesome docs, thanks for adding these! Big fan of the use of "tabs" in the docs to handle v1/v2 examples.
Only nitpick I'd make here is to drop or rephrase the "no longer query parameters" bit (maybe use "instead of"?). But even that's probably personal preference.
##########
solr/core/src/java/org/apache/solr/handler/admin/CreateSnapshotOp.java:
##########
@@ -17,50 +17,29 @@
package org.apache.solr.handler.admin;
-import org.apache.lucene.index.IndexCommit;
-import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.IndexDeletionPolicyWrapper;
-import org.apache.solr.core.SolrCore;
import org.apache.solr.core.snapshots.SolrSnapshotManager;
-import org.apache.solr.core.snapshots.SolrSnapshotMetaDataManager;
+import org.apache.solr.handler.admin.api.SnapshotAPI;
class CreateSnapshotOp implements CoreAdminHandler.CoreAdminOp {
@Override
public void execute(CoreAdminHandler.CallInfo it) throws Exception {
final SolrParams params = it.req.getParams();
- String commitName = params.required().get(CoreAdminParams.COMMIT_NAME);
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
+ final String commitName = params.required().get(CoreAdminParams.COMMIT_NAME);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final SnapshotAPI.CreateSnapshotResponse response =
+ snapshotAPI.createSnapshot(coreName, commitName);
- final String indexDirPath = core.getIndexDir();
- final IndexDeletionPolicyWrapper delPol = core.getDeletionPolicy();
- final IndexCommit ic = delPol.getAndSaveLatestCommit();
- try {
- if (null == ic) {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST, "No index commits to snapshot in core " + cname);
- }
- final SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
- mgr.snapshot(commitName, indexDirPath, ic.getGeneration());
-
- it.rsp.add(CoreAdminParams.CORE, core.getName());
- it.rsp.add(CoreAdminParams.COMMIT_NAME, commitName);
- it.rsp.add(SolrSnapshotManager.INDEX_DIR_PATH, indexDirPath);
- it.rsp.add(SolrSnapshotManager.GENERATION_NUM, ic.getGeneration());
- it.rsp.add(SolrSnapshotManager.FILE_LIST, ic.getFileNames());
- } finally {
- delPol.releaseCommitPoint(ic);
- }
- }
+ it.rsp.add(CoreAdminParams.CORE, response.core);
Review Comment:
[Q] ditto, re: could we use `V2ApiUtils.squashIntoSolrResponseWithoutHeader` here instead of manually copying over values?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1307420832
> If we're still waiting on item 1, I'll go ahead and take a stab at the async code.
Alright, thanks for grabbing that! On item (1), I think the conclusion on that `dev@` thread was that we're good to change the appearance here, since this very much looks like a bug.
I'll take a look sometime today to review what you've added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija merged pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija merged PR #1126:
URL: https://github.com/apache/solr/pull/1126
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1007230522
##########
solr/solr-ref-guide/modules/deployment-guide/pages/backup-restore.adoc:
##########
@@ -234,38 +234,43 @@ http://localhost:8983/solr/gettingstarted/replication?command=restorestatus&wt=x
The status value can be "In Progress", "success" or "failed".
If it failed then an "exception" will also be sent in the response.
-=== Create Snapshot API
+[[create-snapshot-api]]
+== CREATE: Create a Snapshot
The snapshot functionality is different from the backup functionality as the index files aren't copied anywhere.
The index files are snapshotted in the same index directory and can be referenced while taking backups.
You can trigger a snapshot command with an HTTP command like this (replace "techproducts" with the name of the core you are working with):
-.Create Snapshot API Example
-[source,text]
+[.dynamic-tabs]
+--
+[example.tab-pane#v1createsnapshot]
+====
+[.tab-label]*V1 API*
+
+[source,bash]
----
-http://localhost:8983/solr/admin/cores?action=CREATESNAPSHOT&core=techproducts&commitName=commit1
+curl -X POST http://localhost:8983/solr/admin/cores?action=CREATESNAPSHOT&core=techproducts&commitName=commit1
+
----
+====
-The `CREATESNAPSHOT` request parameters are:
+[example.tab-pane#v2createsnapshot]
+====
+[.tab-label]*V2 API*
-`commitName`::
-+
-[%autowidth,frame=none]
-|===
-|Optional |Default: none
-|===
-+
-The name to store the snapshot as.
+With the v2 API, the core and snapshot names are part of the path, no longer query parameters.
Review Comment:
No problem! I actually just copied the structure from elsewhere in the docs, so it wasn't much work on my part. I did go ahead and change the wording since I thought the "instead of" read better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1303901999
1. Awesome, sounds good! The new syntax definitely looks better (and may be easier to parse if someone were using the API from another program), but it's probably a good idea to get consensus first.
2. If we're still waiting on item 1, I'll go ahead and take a stab at it. Also, in reference to the your jira response, I'd be interested in working together on a more general solution too. It sounds like a good way to get familiar with other APIs in Solr.
3. I added three tests just checking the errors response from CoreSnapshotAPI. I started to write some tests (heavily mocked) to verify the response output, but ran into an issue where I couldn't mock the IndexDeletionPolicyWrapper since its a final class. Are there any additional things you can think of that would be worth testing? My current thinking is that we're just trying to test/validate the interface (request, response, errors).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1020335589
##########
solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java:
##########
@@ -0,0 +1,210 @@
+/*
+ * 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 java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.function.Supplier;
+import org.apache.solr.api.JerseyResource;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SolrNamedThreadFactory;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.api.V2ApiUtils;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.logging.MDCLoggingContext;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.tracing.TraceUtils;
+import org.slf4j.MDC;
+
+public abstract class CoreAdminAPIBase extends JerseyResource {
+
+ private static final ExecutorService PARALLEL_EXECUTOR =
Review Comment:
Oh gotcha, not a problem - The way I read your original comment, I thought we were going fully with the static approach first, I can take some blame for my interpretation. I think what you're laying out here makes sense and I hadn't considered the case of multiple Solr Contexts running on a single VM. I took a little bit of time to see how I would register the map/executor, but haven't made any code changes yet. I will attempt to work on it this weekend if you haven't already gotten to it.
Would migrating the status map and executor into a class that can then be used by both the CoreAdminHandler and made injectable for the CoreAdminAPIBase work? That may save us from having to inject multiple things and could potentially get us closer to the future state of what async could look like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1344412977
Hi @Jdurham2843
Just wanted to give you a short update on my lack of progress on this.
It is definitely still on my radar, but I've been a little distracted recently focusing on a performance regression that someone noticed in the JAX-RS framework overall: another committer noticed that the recent addition of the JAX-RS framework has slowed down Solr startup, in some cases quite significantly. That's turned into a big blocker for the JAX-RS work overall: there's even talk of deferring the release of our JAX-RS v2 APIs until that perf regression can be mitigated.
So most of my focus has shifted to that - after all, it's not much good focusing on individual JAX-RS APIs if shipping the whole framework gets delayed đ If you're curious, see SOLR-16531 for more details on all this.
Anyway, that's all to say that I haven't gotten to this PR, but that I haven't forgotten it or let it fall off my radar. I'm just focusing on this performance blocker, and plan to return to this when that's finished. I appreciate all your work here and definitely haven't forgotten it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] sonatype-lift[bot] commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "sonatype-lift[bot] (via GitHub)" <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1103903226
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java:
##########
@@ -493,4 +434,94 @@ public interface CoreAdminOp {
*/
void execute(CallInfo it) throws Exception;
}
+
+ public static class CoreAdminAsyncTracker {
+ private static final int MAX_TRACKED_REQUESTS = 100;
+ public static final String RUNNING = "running";
+ public static final String COMPLETED = "completed";
+ public static final String FAILED = "failed";
+ public final Map<String, Map<String, TaskObject>> requestStatusMap;
+
+ private ExecutorService parallelExecutor =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ 50, new SolrNamedThreadFactory("parallelCoreAdminAPIBaseExecutor"));
+
+ public CoreAdminAsyncTracker() {
+ HashMap<String, Map<String, TaskObject>> map = new HashMap<>(3, 1.0f);
+ map.put(RUNNING, Collections.synchronizedMap(new LinkedHashMap<>()));
+ map.put(COMPLETED, Collections.synchronizedMap(new LinkedHashMap<>()));
+ map.put(FAILED, Collections.synchronizedMap(new LinkedHashMap<>()));
+ requestStatusMap = Collections.unmodifiableMap(map);
+ }
+
+ public void shutdown() {
+ ExecutorUtil.shutdownAndAwaitTermination(parallelExecutor);
+ }
+
+ public Map<String, TaskObject> getRequestStatusMap(String key) {
+ return requestStatusMap.get(key);
+ }
+
+ public ExecutorService getParallelExecutor() {
+ return parallelExecutor;
+ }
+
+ /** Helper method to add a task to a tracking type. */
+ public void addTask(String type, TaskObject o, boolean limit) {
+ synchronized (getRequestStatusMap(type)) {
+ if (limit && getRequestStatusMap(type).size() == MAX_TRACKED_REQUESTS) {
+ String key = getRequestStatusMap(type).entrySet().iterator().next().getKey();
+ getRequestStatusMap(type).remove(key);
+ }
+ addTask(type, o);
+ }
+ }
+
+ public void addTask(String type, TaskObject o) {
+ synchronized (getRequestStatusMap(type)) {
+ getRequestStatusMap(type).put(o.taskId, o);
+ }
+ }
+
+ /** Helper method to remove a task from a tracking map. */
+ public void removeTask(String map, String taskId) {
+ synchronized (getRequestStatusMap(map)) {
Review Comment:
<picture><img alt="5% of developers fix this issue" src="https://lift.sonatype.com/api/commentimage/fixrate/5/display.svg"></picture>
<b>*NULLPTR_DEREFERENCE:</b>* `Map CoreAdminHandler$CoreAdminAsyncTracker.getRequestStatusMap(String)` could be null (from the call to `CoreAdminHandler$CoreAdminAsyncTracker.getRequestStatusMap(...)` on line 488) and is dereferenced.
ââ <b>2 similar findings have been found in this PR</b>
<details><summary>đ Expand here to view all instances of this finding</summary><br/>
<div align=\"center\">
| **File Path** | **Line Number** |
| ------------- | ------------- |
| solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java | [471](https://github.com/apache/solr/blob/3d8b1eb13ffc0307ec0a2d066e5e1bd40521e514/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java#L471) |
| solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java | [481](https://github.com/apache/solr/blob/3d8b1eb13ffc0307ec0a2d066e5e1bd40521e514/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java#L481) |
<p><a href="https://lift.sonatype.com/results/github.com/apache/solr/01GS3Z89WXDS3GKQY5ME5F0MAS?t=Infer|NULLPTR_DEREFERENCE" target="_blank">Visit the Lift Web Console</a> to find more details in your report.</p></div></details>
---
<details><summary>âšī¸ Expand to see all <b>@sonatype-lift</b> commands</summary>
You can reply with the following commands. For example, reply with ***@sonatype-lift ignoreall*** to leave out all findings.
| **Command** | **Usage** |
| ------------- | ------------- |
| `@sonatype-lift ignore` | Leave out the above finding from this PR |
| `@sonatype-lift ignoreall` | Leave out all the existing findings from this PR |
| `@sonatype-lift exclude <file\|issue\|path\|tool>` | Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file |
**Note:** When talking to LiftBot, you need to **refresh** the page to see its response.
<sub>[Click here](https://github.com/apps/sonatype-lift/installations/new) to add LiftBot to another repo.</sub></details>
---
<b>Help us improve LIFT! (<i>Sonatype LiftBot external survey</i>)</b>
Was this a good recommendation for you? <sub><small>Answering this survey will not impact your Lift settings.</small></sub>
[â[đ Not relevant](https://www.sonatype.com/lift-comment-rating?comment=388185228&lift_comment_rating=1)â] - [â[đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=388185228&lift_comment_rating=2)â] - [â[đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=388185228&lift_comment_rating=3)â] - [â[đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=388185228&lift_comment_rating=4)â] - [â[đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=388185228&lift_comment_rating=5)â]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1426086081
Alright, my apologies again for the big delay @Jdurham2843, but it looks like SOLR-16531 is near enough to closing out that I'm ready to pick this up again.
Re-reading the conversation here, it sounds like you had addressed all of the sticking points (particularly the executor injection and async handling), and it's ready for a re-review? Lmk if I've overlooked some detail, but otherwise I'll try to take a look in the next workday or two!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1102999453
##########
solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java:
##########
@@ -278,31 +276,28 @@ public enum CoreAdminOperation implements CoreAdminOp {
LISTSNAPSHOTS,
it -> {
final SolrParams params = it.req.getParams();
- String cname = params.required().get(CoreAdminParams.CORE);
+ final String coreName = params.required().get(CoreAdminParams.CORE);
- CoreContainer cc = it.handler.getCoreContainer();
+ final CoreContainer coreContainer = it.handler.getCoreContainer();
+ final SnapshotAPI snapshotAPI = new SnapshotAPI(coreContainer);
- try (SolrCore core = cc.getCore(cname)) {
- if (core == null) {
- throw new SolrException(ErrorCode.BAD_REQUEST, "Unable to locate core " + cname);
- }
+ final SnapshotAPI.ListSnapshotsResponse response = snapshotAPI.listSnapshots(coreName);
- SolrSnapshotMetaDataManager mgr = core.getSnapshotMetaDataManager();
- @SuppressWarnings({"rawtypes"})
- NamedList result = new NamedList();
- for (String name : mgr.listSnapshots()) {
- Optional<SnapshotMetaData> metadata = mgr.getSnapshotMetaData(name);
- if (metadata.isPresent()) {
- NamedList<String> props = new NamedList<>();
- props.add(
- SolrSnapshotManager.GENERATION_NUM,
- String.valueOf(metadata.get().getGenerationNumber()));
- props.add(SolrSnapshotManager.INDEX_DIR_PATH, metadata.get().getIndexDirPath());
- result.add(name, props);
- }
- }
- it.rsp.add(SolrSnapshotManager.SNAPSHOTS_INFO, result);
+ final NamedList<Object> snapshotsResult = new NamedList<>();
Review Comment:
There were no objections on the [dev@ mailing list thread](https://lists.apache.org/thread/37vcxnprxlrnnqf8wnoms4f66xv0mb85), so I think we're fine to treat this as a bug and not a breaking change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] gerlowskija commented on a diff in pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by "gerlowskija (via GitHub)" <gi...@apache.org>.
gerlowskija commented on code in PR #1126:
URL: https://github.com/apache/solr/pull/1126#discussion_r1108554248
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1095,26 +1095,21 @@ protected void configure() {
.to(SolrNodeKeyPair.class)
.in(Singleton.class);
}
+ })
+ .register(
+ new AbstractBinder() {
+ @Override
+ protected void configure() {
+ bindFactory(
+ new InjectionFactories.SingletonFactory<>(
+ coreAdminHandler.getCoreAdminAsyncTracker()))
+ .to(CoreAdminHandler.CoreAdminAsyncTracker.class)
+ .in(Singleton.class);
+ }
});
jerseyAppHandler = new ApplicationHandler(containerHandlers.getJerseyEndpoints());
}
- // Do Node setup logic after all handlers have been registered.
Review Comment:
I added this back in. Let's discuss if there was a reason it was removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] Jdurham2843 commented on pull request #1126: SOLR-16462: Create v2 equivalent of v1 'CREATESNAPSHOT', 'LISTSNAPSHOT' and 'DELETESNAPSHOT' (core level)
Posted by GitBox <gi...@apache.org>.
Jdurham2843 commented on PR #1126:
URL: https://github.com/apache/solr/pull/1126#issuecomment-1304874435
1. (pending)
2. I created a new CoreAdminAPIBase class to house the async handling code. The class takes the async code from CoreAdminHandler and wraps it in handle method. The handle method then takes it some parameters from each endpoint along with a Supplier<T extends JerseyResponse>. This appears to work the same as the original V1 APIs async code. However, injecting a SolrQueryRequest and SolrQueryResponse has led to a MultiException stack error. See below for the output.
3. I figured out a way to write the response validation tests without needing to mock. It turns out everything I needed was already in the Test's base class. I just had to call a few methods to setup the TestHarness.
Let me know what you think!
MultiException error when calling new V2 methods:
```
Nov 06, 2022 7:01:24 PM org.glassfish.jersey.internal.Errors logErrors
WARNING: The following warnings have been detected: WARNING: HK2 failure has been detected in a code that does not run in an active Jers
ey Error scope.
WARNING: Unknown HK2 failure detected:
MultiException stack 1 of 3
java.lang.NullPointerException
at org.glassfish.jersey.server.internal.process.RequestProcessingConfigurator$ContainerRequestFactory.get(RequestProcessingConfi
gurator.java:67)
at org.glassfish.jersey.server.internal.process.RequestProcessingConfigurator$ContainerRequestFactory.get(RequestProcessingConfi
gurator.java:56)
at org.glassfish.jersey.inject.hk2.SupplierFactoryBridge.provide(SupplierFactoryBridge.java:76)
at org.jvnet.hk2.internal.FactoryCreator.create(FactoryCreator.java:129)
at org.jvnet.hk2.internal.SystemDescriptor.create(SystemDescriptor.java:463)
at org.glassfish.jersey.inject.hk2.RequestContext.findOrCreate(RequestContext.java:59)
at org.jvnet.hk2.internal.Utilities.createService(Utilities.java:2102)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:93)
at org.jvnet.hk2.internal.ServiceLocatorImpl.getService(ServiceLocatorImpl.java:679)
at org.jvnet.hk2.internal.ThreeThirtyResolver.resolve(ThreeThirtyResolver.java:54)
at org.jvnet.hk2.internal.ClazzCreator.resolve(ClazzCreator.java:188)
at org.jvnet.hk2.internal.ClazzCreator.resolveAllDependencies(ClazzCreator.java:205)
at org.jvnet.hk2.internal.ClazzCreator.create(ClazzCreator.java:334)
at org.jvnet.hk2.internal.SystemDescriptor.create(SystemDescriptor.java:463)
at org.jvnet.hk2.internal.PerLookupContext.findOrCreate(PerLookupContext.java:46)
at org.jvnet.hk2.internal.Utilities.createService(Utilities.java:2102)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:93)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:67)
at org.jvnet.hk2.internal.FactoryCreator.dispose(FactoryCreator.java:151)
at org.jvnet.hk2.internal.SystemDescriptor.dispose(SystemDescriptor.java:518)
at org.glassfish.jersey.inject.hk2.RequestContext.lambda$findOrCreate$0(RequestContext.java:60)
at org.glassfish.jersey.internal.inject.ForeignDescriptorImpl.dispose(ForeignDescriptorImpl.java:63)
at org.glassfish.jersey.inject.hk2.Hk2RequestScope$Instance.remove(Hk2RequestScope.java:126)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at org.glassfish.jersey.inject.hk2.Hk2RequestScope$Instance.release(Hk2RequestScope.java:143)
at org.glassfish.jersey.process.internal.RequestScope.release(RequestScope.java:246)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:267)
at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:234)
at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:684)
at org.apache.solr.api.V2HttpCall.invokeJerseyRequest(V2HttpCall.java:371)
at org.apache.solr.api.V2HttpCall.invokeJerseyRequest(V2HttpCall.java:338)
at org.apache.solr.api.V2HttpCall.executeCoreRequest(V2HttpCall.java:452)
at org.apache.solr.servlet.HttpSolrCall.call(HttpSolrCall.java:554)
at org.apache.solr.servlet.SolrDispatchFilter.dispatch(SolrDispatchFilter.java:251)
at org.apache.solr.servlet.SolrDispatchFilter.lambda$doFilter$0(SolrDispatchFilter.java:219)
at org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2(ServletUtils.java:257)
at org.apache.solr.servlet.ServletUtils.rateLimitRequest(ServletUtils.java:227)
at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:214)
at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:196)
at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:201)
at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1626)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:552)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:600)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1440)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:505)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1355)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:191)
at org.eclipse.jetty.server.handler.InetAccessHandler.handle(InetAccessHandler.java:177)
at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:322)
at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:772)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.Server.handle(Server.java:516)
at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:487)
at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:732)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:479)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131)
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:409)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
at java.base/java.lang.Thread.run(Thread.java:829)
MultiException stack 2 of 3
java.lang.IllegalArgumentException: While attempting to resolve the dependencies of org.apache.solr.jersey.InjectionFactories$SolrQueryR
equestFactory errors were found
at org.jvnet.hk2.internal.ClazzCreator.resolveAllDependencies(ClazzCreator.java:224)
at org.jvnet.hk2.internal.ClazzCreator.create(ClazzCreator.java:334)
at org.jvnet.hk2.internal.SystemDescriptor.create(SystemDescriptor.java:463)
at org.jvnet.hk2.internal.PerLookupContext.findOrCreate(PerLookupContext.java:46)
at org.jvnet.hk2.internal.Utilities.createService(Utilities.java:2102)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:93)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:67)
at org.jvnet.hk2.internal.FactoryCreator.dispose(FactoryCreator.java:151)
at org.jvnet.hk2.internal.SystemDescriptor.dispose(SystemDescriptor.java:518)
at org.glassfish.jersey.inject.hk2.RequestContext.lambda$findOrCreate$0(RequestContext.java:60)
at org.glassfish.jersey.internal.inject.ForeignDescriptorImpl.dispose(ForeignDescriptorImpl.java:63)
at org.glassfish.jersey.inject.hk2.Hk2RequestScope$Instance.remove(Hk2RequestScope.java:126)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at org.glassfish.jersey.inject.hk2.Hk2RequestScope$Instance.release(Hk2RequestScope.java:143)
at org.glassfish.jersey.process.internal.RequestScope.release(RequestScope.java:246)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:267)
at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:234)
at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:684)
at org.apache.solr.api.V2HttpCall.invokeJerseyRequest(V2HttpCall.java:371)
at org.apache.solr.api.V2HttpCall.invokeJerseyRequest(V2HttpCall.java:338)
at org.apache.solr.api.V2HttpCall.executeCoreRequest(V2HttpCall.java:452)
at org.apache.solr.servlet.HttpSolrCall.call(HttpSolrCall.java:554)
at org.apache.solr.servlet.SolrDispatchFilter.dispatch(SolrDispatchFilter.java:251)
at org.apache.solr.servlet.SolrDispatchFilter.lambda$doFilter$0(SolrDispatchFilter.java:219)
at org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2(ServletUtils.java:257)
at org.apache.solr.servlet.ServletUtils.rateLimitRequest(ServletUtils.java:227)
at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:214)
at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:196)
at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:201)
at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1626)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:552)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:600)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1440)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:505)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1355)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:191)
at org.eclipse.jetty.server.handler.InetAccessHandler.handle(InetAccessHandler.java:177)
at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:322)
at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:772)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.Server.handle(Server.java:516)
at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:487)
at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:732)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:479)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131)
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:409)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
at java.base/java.lang.Thread.run(Thread.java:829)
MultiException stack 3 of 3
java.lang.IllegalStateException: Unable to perform operation: resolve on org.apache.solr.jersey.InjectionFactories$SolrQueryRequestFacto
ry
at org.jvnet.hk2.internal.ClazzCreator.create(ClazzCreator.java:363)
at org.jvnet.hk2.internal.SystemDescriptor.create(SystemDescriptor.java:463)
at org.jvnet.hk2.internal.PerLookupContext.findOrCreate(PerLookupContext.java:46)
at org.jvnet.hk2.internal.Utilities.createService(Utilities.java:2102)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:93)
at org.jvnet.hk2.internal.ServiceHandleImpl.getService(ServiceHandleImpl.java:67)
at org.jvnet.hk2.internal.FactoryCreator.dispose(FactoryCreator.java:151)
at org.jvnet.hk2.internal.SystemDescriptor.dispose(SystemDescriptor.java:518)
at org.glassfish.jersey.inject.hk2.RequestContext.lambda$findOrCreate$0(RequestContext.java:60)
at org.glassfish.jersey.internal.inject.ForeignDescriptorImpl.dispose(ForeignDescriptorImpl.java:63)
at org.glassfish.jersey.inject.hk2.Hk2RequestScope$Instance.remove(Hk2RequestScope.java:126)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at org.glassfish.jersey.inject.hk2.Hk2RequestScope$Instance.release(Hk2RequestScope.java:143)
at org.glassfish.jersey.process.internal.RequestScope.release(RequestScope.java:246)
at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:267)
at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:234)
at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:684)
at org.apache.solr.api.V2HttpCall.invokeJerseyRequest(V2HttpCall.java:371)
at org.apache.solr.api.V2HttpCall.invokeJerseyRequest(V2HttpCall.java:338)
at org.apache.solr.api.V2HttpCall.executeCoreRequest(V2HttpCall.java:452)
at org.apache.solr.servlet.HttpSolrCall.call(HttpSolrCall.java:554)
at org.apache.solr.servlet.SolrDispatchFilter.dispatch(SolrDispatchFilter.java:251)
at org.apache.solr.servlet.SolrDispatchFilter.lambda$doFilter$0(SolrDispatchFilter.java:219)
at org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2(ServletUtils.java:257)
at org.apache.solr.servlet.ServletUtils.rateLimitRequest(ServletUtils.java:227)
at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:214)
at org.apache.solr.servlet.SolrDispatchFilter.doFilter(SolrDispatchFilter.java:196)
at org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:201)
at org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1626)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:552)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:600)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1440)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:505)
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1355)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:191)
at org.eclipse.jetty.server.handler.InetAccessHandler.handle(InetAccessHandler.java:177)
at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.rewrite.handler.RewriteHandler.handle(RewriteHandler.java:322)
at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:772)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.Server.handle(Server.java:516)
at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:487)
at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:732)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:479)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131)
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:409)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
at java.base/java.lang.Thread.run(Thread.java:829)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org