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/24 10:52:49 UTC
[GitHub] [solr] joshgog opened a new pull request, #1115: Created V2 equivalent of REPLACENODE
joshgog opened a new pull request, #1115:
URL: https://github.com/apache/solr/pull/1115
https://issues.apache.org/jira/browse/SOLR-11028
<!--
_(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
Create a v2 equivalent of 'REPLACENODE command.
# Solution
Created an API that uses JAX-RS annotations.
# Tests
The PR is a WIP. Test cases not implemented.
# 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.
- [x] 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.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] 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] joshgog commented on pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1297522684
Quite helpful information. Will decide between the Jersey test and the remote message test
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1003984443
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
I used the path as documented in the proposed api design document [here](https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA/edit#gid=1549066254)
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1305947078
> the `nodes/SingleNode` is already there, so we are still following this consistency.
Doing it for nodes and not commands or other things is the definition of inconsistency đ
We follow it here for the node-name, yes. But the idea isn't to do this once-per-path and call it a day, it's to do it everywhere we expose a noun or resource in our API paths. The consistency of design across APIs is what helps users reason about what an API will likely be for some operation they're unfamiliar with. If a user is familiar with other APIs and has seen Solr use this pattern for nodes, collections, cores, shards, replicas, replica properties, cluster properties, fields, fieldTypes, requestHandlers, and on and on .... well then they'll probably expect something similar here.
IMO, The question shouldn't be whether we can get by without this in the path. The question should be: what does omitting `/commands` get us that makes taking the hit on consistency worth 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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005275521
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, targetNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(
Review Comment:
> Unless you had a particular reason for excluding this from your RequestBody POJO below and from this message-creation here, that should probably be added?
Will add that
> Also, mostly unrelated to this PR, I notice that the [REPLACENODE ref-guide docs](https://solr.apache.org/guide/8_11/cluster-node-management.html#replacenode-parameters) mention a few parameters (parallel, timeout, etc.) that I don't see used in the code (either with or without your PR). Are those docs just plain wrong? Or am I missing something?
You are right, the docs is not in sync with the code.. To add them to the RequestBody POJO will we have to write code for their functionality e.g for parallel parameter `If this flag is set to true, all replicas are created in separate threads`
--
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] joshgog commented on pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1292427885
I have added a test that extends SolrTestCaseJ4 but not sure if its exhaustive
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005251953
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
Review Comment:
Sure
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1009697177
##########
solr/core/src/test/org/apache/solr/handler/admin/api/ReplaceNodeAPITest.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Optional;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.cloud.OverseerSolrResponse;
+import org.apache.solr.cloud.api.collections.DistributedCollectionConfigSetCommandRunner;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Unit tests for {@link ReplaceNodeAPI} */
+public class ReplaceNodeAPITest extends SolrTestCaseJ4 {
+
+ private CoreContainer mockCoreContainer;
+ private static final String sourceNodeName = "demoSourceNode";
+ private static final String targetNodeName = "demoTargetNode";
+ public static final String async = "async";
+ private static final boolean waitForFinalState = false;
+ private SolrQueryRequest mockQueryRequest;
+ private SolrQueryResponse queryResponse;
+ private ReplaceNodeAPI replaceNodeApi;
+ private DistributedCollectionConfigSetCommandRunner mockCommandRunner;
+
+ @BeforeClass
+ public static void ensureWorkingMockito() {
+ assumeWorkingMockito();
+ }
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+
+ mockCoreContainer = mock(CoreContainer.class);
+ mockCommandRunner = mock(DistributedCollectionConfigSetCommandRunner.class);
+ when(mockCoreContainer.getDistributedCollectionCommandRunner())
+ .thenReturn(Optional.of(mockCommandRunner));
+ when(mockCommandRunner.runCollectionCommand(any(), any(), anyLong()))
+ .thenReturn(new OverseerSolrResponse(new NamedList<>()));
+ mockQueryRequest = mock(SolrQueryRequest.class);
+ queryResponse = new SolrQueryResponse();
+ replaceNodeApi = new ReplaceNodeAPI(mockCoreContainer, mockQueryRequest, queryResponse);
+ }
+
+ @Test
+ public void testSuccessfulReplaceNodeCommand() throws Exception {
Review Comment:
[-1] I think we should either re-work this test a bit to assert on something more meaningful (e.g. the remote-message that gets created from all the API parameters), or we should lean exclusively on the JerseyTest you mentioned in some comments.
Either approach (or some third option if you've got other ideas) is totally fine with me. But as-is at least this test just doesn't validate all that much.
If you want to go with the first option (reworking this to assert on the remote-message), then AddReplicaPropertyAPITest might be a good reference. That test makes an API call and uses a special type of mock called an "ArgumentCaptor" to record the "remote message", so that the test can assert on it later.
The key bits in AddReplicaPropertyAPITest are:
```
messageCapturer = ArgumentCaptor.forClass(ZkNodeProps.class);
```
(creates the argument-captor)
```
addReplicaPropApi.addReplicaProperty(
"someColl", "someShard", "someReplica", "somePropName", ANY_REQ_BODY);
verify(mockCommandRunner).runCollectionCommand(messageCapturer.capture(), any(), anyLong());
```
(calls the new API and "captures" or records the remote-message)
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1003984443
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
I used the path as documented in the proposed api design document [here](https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA/edit#gid=1549066254)
Will updated as suggested
--
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 #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1006066767
##########
solr/modules/hadoop-auth/src/java/org/apache/solr/security/hadoop/RequestContinuesRecorderAuthenticationHandler.java:
##########
@@ -25,9 +25,10 @@
import org.apache.hadoop.security.authentication.server.AuthenticationHandler;
import org.apache.hadoop.security.authentication.server.AuthenticationToken;
-/**
- * {@link AuthenticationHandler} that delegates to another {@link AuthenticationHandler} and records
- * the response of managementOperation (which indicates whether the request should continue or not).
+/*
+ * {@link AuthenticationHandler} that delegates to another {@link AuthenticationHandler}
+ * and records the response of managementOperation (which indicates whether the request
+ * should continue or not).
*/
public class RequestContinuesRecorderAuthenticationHandler implements AuthenticationHandler {
Review Comment:
*[AlmostJavadoc](https://errorprone.info/bugpattern/AlmostJavadoc):* This comment contains Javadoc or HTML tags, but isn't started with a double asterisk (/**); is it meant to be Javadoc?
---
```suggestion
/**
```
---
<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=348391765&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=348391765&lift_comment_rating=5) ]
##########
solr/prometheus-exporter/src/java/org/apache/solr/prometheus/exporter/MetricsQueryTemplate.java:
##########
@@ -23,20 +23,21 @@
import java.util.regex.Pattern;
public class MetricsQueryTemplate {
- /**
- * A regex with named groups is used to match template references to template + vars using the
- * basic pattern:
- *
- * <p>$jq:<TEMPLATE>( <UNIQUE>, <KEYSELECTOR>, <METRIC>, <TYPE> )
- *
- * <p>For instance,
- *
- * <p>$jq:core(requests_total, endswith(".requestTimes"), count, COUNTER)
- *
- * <p>TEMPLATE = core UNIQUE = requests_total (unique suffix for this metric, results in a metric
- * named "solr_metrics_core_requests_total") KEYSELECTOR = endswith(".requestTimes") (filter to
- * select the specific key for this metric) METRIC = count TYPE = COUNTER
- */
+ /*
+ A regex with named groups is used to match template references to template + vars using the basic pattern:
+
+ $jq:<TEMPLATE>( <UNIQUE>, <KEYSELECTOR>, <METRIC>, <TYPE> )
+
+ For instance,
+
+ $jq:core(requests_total, endswith(".requestTimes"), count, COUNTER)
+
+ TEMPLATE = core
+ UNIQUE = requests_total (unique suffix for this metric, results in a metric named "solr_metrics_core_requests_total")
+ KEYSELECTOR = endswith(".requestTimes") (filter to select the specific key for this metric)
+ METRIC = count
+ TYPE = COUNTER
+ */
private static final Pattern matchJqTemplate =
Review Comment:
*[AlmostJavadoc](https://errorprone.info/bugpattern/AlmostJavadoc):* This comment contains Javadoc or HTML tags, but isn't started with a double asterisk (/**); is it meant to be Javadoc?
---
```suggestion
/**
```
---
<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=348391772&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=348391772&lift_comment_rating=5) ]
##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/TimeDifferencingEvaluator.java:
##########
@@ -59,6 +59,14 @@ public Object doWork(Object... values) throws IOException {
Number lagValue = 1;
if (1 == values.length) {
+ if (!(timeseriesValues instanceof List<?>)) {
Review Comment:
*[BadInstanceof](https://errorprone.info/bugpattern/BadInstanceof):* `timeseriesValues` is an instance of List<Number> which is a subtype of List<?>, so this is equivalent to a null check.
---
```suggestion
if (timeseriesValues == null) {
```
---
<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=348391732&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=348391732&lift_comment_rating=5) ]
##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/TimeDifferencingEvaluator.java:
##########
@@ -106,7 +114,7 @@ public Object doWork(Object... values) throws IOException {
if (2 == values.length) {
lagValue = (Number) values[1];
- if (lagValue == null) {
+ if (!(lagValue instanceof Number)) {
Review Comment:
*[BadInstanceof](https://errorprone.info/bugpattern/BadInstanceof):* `lagValue` is an instance of Number which is a subtype of Number, so this is equivalent to a null check.
---
```suggestion
if (lagValue == null) {
```
---
<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=348391771&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=348391771&lift_comment_rating=5) ]
##########
solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/TimeDifferencingEvaluator.java:
##########
@@ -70,7 +78,7 @@ public Object doWork(Object... values) throws IOException {
}
if (2 == values.length) {
lagValue = (Number) values[1];
- if (lagValue == null) {
+ if (!(lagValue instanceof Number)) {
Review Comment:
*[BadInstanceof](https://errorprone.info/bugpattern/BadInstanceof):* `lagValue` is an instance of Number which is a subtype of Number, so this is equivalent to a null check.
---
```suggestion
if (lagValue == null) {
```
---
<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=348391790&lift_comment_rating=1) ] - [ [đ Won't fix](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=2) ] - [ [đ Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=3) ] - [ [đ Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=348391790&lift_comment_rating=4) ] - [ [đ Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=348391790&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] joshgog commented on pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1310585796
Thank you for the merge
--
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] joshgog commented on pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1292436699
Initially I had tried using JerseyTest but the test failed due to server shutdown.
```
2> INFO: Started listener bound to [localhost:9998]
2> Oct 26, 2022 9:23:11 PM org.glassfish.grizzly.http.server.HttpServer start
2> INFO: [HttpServer] Started.
2> Oct 26, 2022 9:23:12 PM org.glassfish.grizzly.http.server.NetworkListener shutdownNow
2> INFO: Stopped listener bound to [localhost:9998]
```
The code is
```
public class ReplaceNodeAPITest extends JerseyTest {
private CoreContainer mockCoreContainer;
private static final String sourceNodeName = "demoSourceNode";
private static final String targetNodeName = "demoTargetNode";
private static final boolean waitForFinalState = false;
private SolrQueryRequest mockQueryRequest;
private SolrQueryResponse queryResponse;
@BeforeClass
public static void ensureWorkingMockito() {
assumeWorkingMockito();
}
protected Application configure() {
resetMocks();
final ResourceConfig config = new ResourceConfig();
config.register(ReplaceNodeAPI.class);
config.register(SolrJacksonMapper.class);
config.register(
new AbstractBinder() {
@Override
protected void configure() {
bindFactory(new InjectionFactories.SingletonFactory<>(mockCoreContainer))
.to(CoreContainer.class)
.in(Singleton.class);
}
});
config.register(
new AbstractBinder() {
@Override
protected void configure() {
bindFactory(InjectionFactories.SolrQueryRequestFactory.class)
.to(SolrQueryRequest.class)
.in(RequestScoped.class);
}
});
config.register(
new AbstractBinder() {
@Override
protected void configure() {
bindFactory(InjectionFactories.SolrQueryResponseFactory.class)
.to(SolrQueryResponse.class)
.in(RequestScoped.class);
}
});
return config;
}
@Test
public void testSuccessfulReplaceNodeCommand() throws Exception {
final String expectedJson = "{\"responseHeader\":{\"status\":0,\"QTime\":0}}";
final CollectionsHandler collectionsHandler = mock(CollectionsHandler.class);
when(mockCoreContainer.getCollectionsHandler()).thenReturn(collectionsHandler);
ReplaceNodeAPI.ReplaceNodeRequestBody requestBody = new ReplaceNodeAPI.ReplaceNodeRequestBody(targetNodeName, waitForFinalState);
Entity<ReplaceNodeRequestBody> entity = Entity.entity(requestBody, MediaType.APPLICATION_JSON);
final Response response =
target("cluster/nodes/" + sourceNodeName + "/commands/replace").request().post(entity);
final String jsonBody = response.readEntity(String.class);
assertEquals(200, response.getStatus());
assertEquals("application/json", response.getHeaders().getFirst("Content-type"));
assertEquals(expectedJson, "{\"responseHeader\":{\"status\":0,\"QTime\":0}}");
}
private void resetMocks() {
mockCoreContainer = mock(CoreContainer.class);
}
}
```
--
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 #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005806358
##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1791,14 +1794,17 @@ public Map<String, Object> execute(
REPLACENODE_OP(
REPLACENODE,
(req, rsp, h) -> {
- return copy(
- req.getParams(),
- null,
- "source", // legacy
- "target", // legacy
- WAIT_FOR_FINAL_STATE,
- CollectionParams.SOURCE_NODE,
- CollectionParams.TARGET_NODE);
+ final SolrParams params = req.getParams();
+ final RequiredSolrParams requiredParams = req.getParams().required();
+ final ReplaceNodeAPI.ReplaceNodeRequestBody requestBody =
+ new ReplaceNodeAPI.ReplaceNodeRequestBody();
+ requestBody.targetNodeName = params.get(TARGET_NODE);
+ requestBody.waitForFinalState = params.getBool(WAIT_FOR_FINAL_STATE, false);
Review Comment:
[0] Prior to this PR, if the user didn't explicitly specify a waitForFinalState value, the created "message" wouldn't contain that key at all.
The use of `getBool(key, defaultVal)` here means that the constructed RNRB will _always_ have a waitForFinalState value that gets added to the "message" that the v2 code builds. This isn't a problem necessarily; the docs say that this parameter defaults to "false" - so this shouldn't change any behavior. But it is something to keep an eye on if test failures crop up later.
If you wanted to be more conservative though, you could drop the default-val here to keep closer in line with the existing behavior.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String nodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
+ remoteMessage.put(WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
+ remoteMessage.put(QUEUE_OPERATION, CollectionAction.REPLACENODE.toLower());
+
+ return new ZkNodeProps(remoteMessage);
+ }
+
+ public static class ReplaceNodeRequestBody implements JacksonReflectMapWriter {
+
+ public ReplaceNodeRequestBody() {}
+
+ public ReplaceNodeRequestBody(String targetNodeName, Boolean waitForFinalState) {
+ this.targetNodeName = targetNodeName;
+ this.waitForFinalState = waitForFinalState;
+ }
+
+ @Schema(description = "The name of the targetNode.")
+ @JsonProperty("targetNodeName")
+ public String targetNodeName;
+
+ @JsonProperty("waitForFinalState")
Review Comment:
[0] The `@Schema` annotations you see on this code end up used as descriptions in the OpenAPI spec that Solr generates as part of its build.
If you're not familiar with OpenAPI, it's really awesome! It provides tooling you can use to generate a "spec" - a machine-readable description of your API. And then other tooling can take that spec and do all sorts of things with it, like generating clients for various languages.
Right now, Solr's set up to generate a spec (run `./gradlew resolve` and take a look at `solr/core/build/generated/openapi/openapi.json`). We haven't started using the spec yet, but it's on my roadmap to set us up to generate Solr clients for Python, golang, etc.
Anyway, this is all to say: these `@Schema` annotations don't seem like they do much now, but they'll be super helpful to have in place once we start using the spec.
You don't need to have them everywhere if you're not sure what to say about certain parameters. But if you want to include them, you can always copy the snippet in the ref guide that describes the parameter.
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String nodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
+ remoteMessage.put(WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
+ remoteMessage.put(QUEUE_OPERATION, CollectionAction.REPLACENODE.toLower());
+
+ return new ZkNodeProps(remoteMessage);
+ }
+
+ public static class ReplaceNodeRequestBody implements JacksonReflectMapWriter {
+
+ public ReplaceNodeRequestBody() {}
+
+ public ReplaceNodeRequestBody(String targetNodeName, Boolean waitForFinalState) {
+ this.targetNodeName = targetNodeName;
+ this.waitForFinalState = waitForFinalState;
+ }
+
Review Comment:
[-1] v1 REPLACENODE supports the "async" parameter, so we should probably add that to the RequestBody as well.
(`CollectionsHandler.submitCollectionApiCommand`, which you use above, already has logic built around the parameter, so all we should have to do is add it to the RequestBody here, and then make sure it gets added to the message in `createRemoteMessage` above.)
##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1791,14 +1794,17 @@ public Map<String, Object> execute(
REPLACENODE_OP(
REPLACENODE,
(req, rsp, h) -> {
- return copy(
- req.getParams(),
- null,
- "source", // legacy
Review Comment:
[-1] I'm fine with dropping the deprecated old parameter names in this PR, but if we drop them here, we should also drop the code that checks for them in [ReplaceNodeCmd](https://github.com/apache/solr/blob/c99af207c761ec34812ef1cc3054eb2804b7448b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java#L67)
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String nodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
Review Comment:
[0] If the user doesn't provide a request-body, e.g.
```
curl -ilk -X POST "http://localhost:8983/api/cluster/nodes/localhost:7574_solr/commands/replace"
```
then this line will cause a NullPointerException.
I'm a little torn on this - I think it'd technically be "correct" if we forced users to provide an empty JSON object (`{}`) as the request body even if they don't want to specify any of the optional parameters. But in practice - idk, it seems like it'd be a pain that we could save our users.
What do you think of surrounding these lines with a `if (requestBody != null)` check?
Also, this would be a great this to have a test for later on when you get to that point.
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1307789226
Ah, I was just coming back here to close the loop but I see Houston beat me to it.
There is a distinction between the APIs that include `/commands` in the path and those that don't, but Houston pointed out (correctly I think) that the distinction might be a bit subtle to expect our users to pick up on. I suspect David feels similarly.
I think we should go with the majority opinion and drop `/commands` from the path here and elsewhere.
> I would be willing to go through and help make the changes I'm suggesting (removing commands/ throughout the API). I wouldn't put that work on anyone else!
I appreciate it, but no need. The change needed here is small and I can just make it now while I've got it pulled up. That just leaves the spreadsheet, which should be easy enough to CTRL-F on. I'd appreciate your review on the other tabs in that spreadsheet for sure though, if you've got a few @HoustonPutman.
With that resolved this will be back on my list of things to test and commit.
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija merged PR #1115:
URL: https://github.com/apache/solr/pull/1115
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1297503312
> The error received is 500.
Make sure you register `CatchAllExceptionMapper.class` in your JerseyTest's `configure()` method...that makes it much easier to see what's going on here. When I add that to your test class, put a debugger break-point in CatchAllExceptionMapper and re-run, I get the following error:
```
Solr instance is not running in SolrCloud mode.
```
Which makes sense - one of the first things that ReplaceNodeAPI does is call `fetchAndValidateZooKeeperAwareCoreContainer`, and the `mockCoreContainer` your test uses isn't configured to say that it's ZK-aware. I fixed that by adding the line:
```
when(mockCoreContainer.isZooKeeperAware()).thenReturn(true);
```
to the test, which lets us get further until we hit a different mock related error later on. But, that's progress.
Anyway, hope that helps!
--
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] joshgog commented on a diff in pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1017949902
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,139 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String sourceNodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(sourceNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ /** TODO Test null check */
+ if (requestBody != null) {
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
+ remoteMessage.put(WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
Review Comment:
>
Looking at the commits to better understand 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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005412245
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
I agree. Having targetNodeName in the path wasn't ideal
--
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] noblepaul commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
noblepaul commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1003210468
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
why do we have a`/nodes` prefix? it's confusing
it should be a `/cluster` prefix as in `/cluster/nodes`
--
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 #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1004828107
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
**TL;DR** I like Noble's suggestion to switch to `/cluster/nodes/nodeName/...`. I'd say go with that unless you have any objections Josh?
Additional Context:
Yeah, I pointed Josh at the v2 API syntax proposed in that spreadsheet. There was some debate there about how to handle this and other "node related" APIs. In particular we struggled to come up with a syntax that made sense for Solr's weird mix of node-local (e.g. the v1 `/admin/info/logging`) and truly distributed (`/admin/collections?action=REPLACENODE`) APIs related to nodes. See the previous discussion [here](https://docs.google.com/spreadsheets/d/1HAoBBFPpSiT8mJmgNZKkZAPwfCfPvlc08m5jz3fQBpA/edit?disco=AAAAbSSbh5M).
--
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] joshgog commented on pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1293004331
> Initially I had tried using JerseyTest but the test failed due to server shutdown.
>
> ```
> 2> INFO: Started listener bound to [localhost:9998]
> 2> Oct 26, 2022 9:23:11 PM org.glassfish.grizzly.http.server.HttpServer start
> 2> INFO: [HttpServer] Started.
> 2> Oct 26, 2022 9:23:12 PM org.glassfish.grizzly.http.server.NetworkListener shutdownNow
> 2> INFO: Stopped listener bound to [localhost:9998]
> ```
>
> The code is
>
> ```
> public class ReplaceNodeAPITest extends JerseyTest {
>
> private CoreContainer mockCoreContainer;
> private static final String sourceNodeName = "demoSourceNode";
> private static final String targetNodeName = "demoTargetNode";
> private static final boolean waitForFinalState = false;
> private SolrQueryRequest mockQueryRequest;
> private SolrQueryResponse queryResponse;
>
> @BeforeClass
> public static void ensureWorkingMockito() {
> assumeWorkingMockito();
> }
>
> protected Application configure() {
> resetMocks();
> final ResourceConfig config = new ResourceConfig();
> config.register(ReplaceNodeAPI.class);
> config.register(SolrJacksonMapper.class);
> config.register(
> new AbstractBinder() {
> @Override
> protected void configure() {
> bindFactory(new InjectionFactories.SingletonFactory<>(mockCoreContainer))
> .to(CoreContainer.class)
> .in(Singleton.class);
> }
> });
> config.register(
> new AbstractBinder() {
> @Override
> protected void configure() {
> bindFactory(InjectionFactories.SolrQueryRequestFactory.class)
> .to(SolrQueryRequest.class)
> .in(RequestScoped.class);
> }
> });
> config.register(
> new AbstractBinder() {
> @Override
> protected void configure() {
> bindFactory(InjectionFactories.SolrQueryResponseFactory.class)
> .to(SolrQueryResponse.class)
> .in(RequestScoped.class);
> }
> });
>
> return config;
> }
>
> @Test
> public void testSuccessfulReplaceNodeCommand() throws Exception {
> final String expectedJson = "{\"responseHeader\":{\"status\":0,\"QTime\":0}}";
> final CollectionsHandler collectionsHandler = mock(CollectionsHandler.class);
> when(mockCoreContainer.getCollectionsHandler()).thenReturn(collectionsHandler);
> ReplaceNodeAPI.ReplaceNodeRequestBody requestBody = new ReplaceNodeAPI.ReplaceNodeRequestBody(targetNodeName, waitForFinalState);
> Entity<ReplaceNodeRequestBody> entity = Entity.entity(requestBody, MediaType.APPLICATION_JSON);
> final Response response =
> target("cluster/nodes/" + sourceNodeName + "/commands/replace").request().post(entity);
> final String jsonBody = response.readEntity(String.class);
> assertEquals(200, response.getStatus());
> assertEquals("application/json", response.getHeaders().getFirst("Content-type"));
> assertEquals(expectedJson, "{\"responseHeader\":{\"status\":0,\"QTime\":0}}");
> }
>
> private void resetMocks() {
> mockCoreContainer = mock(CoreContainer.class);
>
> }
> }
> ```
The error received is 500. I think problem is caused by the test case params. I'm working on 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] HoustonPutman commented on pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1303995329
Not sure if this was discussed, but can we take `/commands` out of the path for this? `/replace` alone is pretty self-explanatory.
--
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] joshgog commented on pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1308782346
> I'll aim to run tests/'check' on this over the weekend and commit as long as you've done everything here you were looking to?
I'm trying to catch up after a long weekend. I think all is good after the agreement of removing `/command`
--
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 #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005910405
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, targetNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(
Review Comment:
Alright, so, afaict...
* On "main" today, `parallel` and `timeout` are used in ReplaceNodeCmd, where the message gets processed and the actual functionality is coordinated. But those parameters are never passed through, so that part of ReplaceNodeCmd is "dead".
* The initial implementation of REPLACENODE (see commit ae60c7) passed `parallel` through, but never `timeout` AFAICT.
* `parallel` was dropped on commit c9cf57 of JIRA SOLR-11068. There's no discussion there of why to indicate whether it was intentional or an accident.
I created a JIRA to record some of these details and for someone to address at some point: SOLR-16499
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1009697545
##########
solr/core/src/test/org/apache/solr/handler/admin/api/ReplaceNodeAPITest.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Optional;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.cloud.OverseerSolrResponse;
+import org.apache.solr.cloud.api.collections.DistributedCollectionConfigSetCommandRunner;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/** Unit tests for {@link ReplaceNodeAPI} */
+public class ReplaceNodeAPITest extends SolrTestCaseJ4 {
+
+ private CoreContainer mockCoreContainer;
+ private static final String sourceNodeName = "demoSourceNode";
+ private static final String targetNodeName = "demoTargetNode";
+ public static final String async = "async";
+ private static final boolean waitForFinalState = false;
+ private SolrQueryRequest mockQueryRequest;
+ private SolrQueryResponse queryResponse;
+ private ReplaceNodeAPI replaceNodeApi;
+ private DistributedCollectionConfigSetCommandRunner mockCommandRunner;
+
+ @BeforeClass
+ public static void ensureWorkingMockito() {
+ assumeWorkingMockito();
+ }
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+
+ mockCoreContainer = mock(CoreContainer.class);
+ mockCommandRunner = mock(DistributedCollectionConfigSetCommandRunner.class);
+ when(mockCoreContainer.getDistributedCollectionCommandRunner())
+ .thenReturn(Optional.of(mockCommandRunner));
+ when(mockCommandRunner.runCollectionCommand(any(), any(), anyLong()))
+ .thenReturn(new OverseerSolrResponse(new NamedList<>()));
+ mockQueryRequest = mock(SolrQueryRequest.class);
+ queryResponse = new SolrQueryResponse();
+ replaceNodeApi = new ReplaceNodeAPI(mockCoreContainer, mockQueryRequest, queryResponse);
+ }
+
+ @Test
+ public void testSuccessfulReplaceNodeCommand() throws Exception {
+ when(mockCoreContainer.isZooKeeperAware()).thenReturn(true);
+ final String expectedJson = "{\"responseHeader\":{\"status\":0,\"QTime\":0}}";
+ final CollectionsHandler collectionsHandler = mock(CollectionsHandler.class);
+ when(mockCoreContainer.getCollectionsHandler()).thenReturn(collectionsHandler);
Review Comment:
[0] I suspect this line and the one above it are leftover from earlier testing? At the least they're not doing anything right now - I double-checked ReplaceNodeAPI and it never calls `getCollectionsHandler` or anything similar.
Similarly, the `expectedJson` variable isn't used afaict.
--
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] HoustonPutman commented on pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1307468029
So Jason and I talked offline and cleared up our misunderstandings đ I didn't know that the API spreadsheet had multiple tabs đ
Overall I prefer the consistency of the user experience without `commands/`, though I do now understand the distinction between paths with `commands/` and those without.
I'm just one voice in the sea of contributors, so take my opinion as just that. But I would be willing to go through and help make the changes I'm suggesting (removing `commands/` throughout the API). I wouldn't put that work on anyone else!
--
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] HoustonPutman commented on pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1305755994
> 1. **Consistency** Most of the proposed v2 APIs are designed around this pattern of `/pluralGroupName/singleInstance` (e.g. `/collections/techproducts`, `/shards/shard1`, `/fields/text`.) The `/commands` path segment helps us stay consistent with that pattern.
But given your example (`http://localhost:8983/api/cluster/nodes/localhost:7574_solr/commands/replace/`) the `nodes/SingleNode` is already there, so we are still following this consistency.
Maybe the docs in the PR are out of date, but the uniqueness thing is also not an issue because the node name is already in the URL. I understand how the `cores/commands/swap` example makes sense (though it can probably use the same syntax here with `cores/core_name/swap`...), but I think in general its best that we try to leave out `commands` and other filler paths when we can đ
--
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] noblepaul commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
noblepaul commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1003210468
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
why do we have a`/nodes` prefix? it's confusing. There is a already a `/node/*` prefix which does node specific operations
it should be a `/cluster` prefix as in `/cluster/nodes`
--
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 #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1004888461
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, targetNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(
Review Comment:
[0] It looks like the [v1 codepath that created this message](https://github.com/apache/solr/pull/1115/files#diff-582348d44491dcb0ce1dfb169fb544e9e95620b2d0448eb1a1744f4e8dd5a349L1794) also passed through a `waitForFinalState` parameter.
Unless you had a particular reason for excluding this from your RequestBody POJO below and from this message-creation here, that should probably be added?
Also, mostly unrelated to this PR, I notice that the [REPLACENODE ref-guide docs](https://solr.apache.org/guide/8_11/cluster-node-management.html#replacenode-parameters) mention a few parameters (parallel, timeout, etc.) that I don't see used in the code (either with or without your PR). Are those docs just plain wrong? Or am I missing something? đ¤
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
Review Comment:
[0] Things might be a little easier to read if you rename the path-template and variable here to "sourceNodeName"
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
Review Comment:
[0] AFAICT, this code accepts the targetNodeName as a path parameter (see L72-L74, and L54) and in the request body? Was that intentional, or something that was left behind from an earlier iteration?
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
[-1] "targetNodeName" also needs to come out of the path I think. For two reasons:
First - "targetNodeName" is an optional parameter (at least, according to the docs [here](https://solr.apache.org/guide/8_10/cluster-node-management.html#replacenode-parameters)), which can make it confusing to have in the path.
Second - and admittedly this is subjective - but it's less intuitive IMO. Every other part of the path for this API follows this pattern where each path-segment kindof logically belongs (or fits inside of?) the thing that came before. (e.g. A "cluster" has "nodes". One of the nodes it has is "sourceNodeName". That node has certain "commands" that it supports. One of the commands is "replace".) "targetNodeName" doesn't really fit that pattern as easily IMO.
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1305636871
Gonna hold off on merging this for now based on Houston's comment above...
> can we take /commands out of the path for this? /replace alone is pretty self-explanatory.
Happy to remove it if there's consensus, but `/commands` in the path serves a few functions, at least one of which is probably non-obvious:
1. `/commands` is there for better consistency. All over the place in the APIs we have this pattern of `/pluralCollectionName/singleInstance`, whether this be in the form of `/collections/techproducts`, or `/shards/shard1`, or `/fields/fieldName`. "Commands" helps us stay consistent with that pattern - for whatever that's worth.
2. Less-obvious, using `/commands` helps cut down on potential path ambiguity between command names and names of cores, collections, shards, etc. It's less of an issue with this API in particular, but consider how this plays out for commands on the `/cores` path. Without "commands" in the path, each command name is a potential collision with a core name specified by a user. That is: `/api/cores/swap` becomes ambiguous: is a user attempting to swap two cores, or fetch the status of a core called 'swap'.
Anyway, if neither of that sways you @HoustonPutman or @dsmiley , then we can update the API here and others in the spreadsheet more broadly. But figured i'd make a last defense of 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] gerlowskija commented on a diff in pull request #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1014306588
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,139 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.ASYNC;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String sourceNodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(sourceNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ /** TODO Test null check */
+ if (requestBody != null) {
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
+ remoteMessage.put(WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
Review Comment:
[0] Some of these values we're setting on the remote message might actually be null/unset. The existing v1 code to create this 'message' goes out of its way to exclude null/unset values.
I'm going to push a small update here to make sure these values are only added to the message if they're non-null.
--
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 #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005627447
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, targetNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(
Review Comment:
OK, đ , thanks for the double-check on that. Let's ignore them for now.
I'll try to do some digging to see what the story is with those params (were they removed intentionally? accidentally?). But whatever I find there, it'll mostly just be to record in a separate JIRA.
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005885430
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String nodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
+ remoteMessage.put(WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
+ remoteMessage.put(QUEUE_OPERATION, CollectionAction.REPLACENODE.toLower());
+
+ return new ZkNodeProps(remoteMessage);
+ }
+
+ public static class ReplaceNodeRequestBody implements JacksonReflectMapWriter {
+
+ public ReplaceNodeRequestBody() {}
+
+ public ReplaceNodeRequestBody(String targetNodeName, Boolean waitForFinalState) {
+ this.targetNodeName = targetNodeName;
+ this.waitForFinalState = waitForFinalState;
+ }
+
+ @Schema(description = "The name of the targetNode.")
+ @JsonProperty("targetNodeName")
+ public String targetNodeName;
+
+ @JsonProperty("waitForFinalState")
Review Comment:
Great info! Will include the @Schema annotation
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1006387694
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("cluster/nodes/{sourceNodeName}/commands/replace/")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("sourceNodeName")
+ String nodeName,
+ @RequestBody(description = "Contains user provided parameters", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.REPLACENODE,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(SOURCE_NODE, nodeName);
+ remoteMessage.put(TARGET_NODE, requestBody.targetNodeName);
+ remoteMessage.put(WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
+ remoteMessage.put(QUEUE_OPERATION, CollectionAction.REPLACENODE.toLower());
+
+ return new ZkNodeProps(remoteMessage);
+ }
+
+ public static class ReplaceNodeRequestBody implements JacksonReflectMapWriter {
+
+ public ReplaceNodeRequestBody() {}
+
+ public ReplaceNodeRequestBody(String targetNodeName, Boolean waitForFinalState) {
+ this.targetNodeName = targetNodeName;
+ this.waitForFinalState = waitForFinalState;
+ }
+
Review Comment:
Done
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005878146
##########
solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java:
##########
@@ -1791,14 +1794,17 @@ public Map<String, Object> execute(
REPLACENODE_OP(
REPLACENODE,
(req, rsp, h) -> {
- return copy(
- req.getParams(),
- null,
- "source", // legacy
Review Comment:
Sure
--
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 #1115: SOLR-11028: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
gerlowskija commented on PR #1115:
URL: https://github.com/apache/solr/pull/1115#issuecomment-1303955756
Hi @joshgog - just wanted to summarize where we are on this. I think at this point you've addressed everything I (and Noble) brought up in review so far. Unless I'm missing something?
I've pushed one tiny edit a few minutes ago to ensure that ReplaceNodeAPI doesn't add values to its 'remoteMessage' map if they're null, and some tests for that. But otherwise this looks good to me.
I'll aim to run tests/'check' on this over the weekend and commit as long as you've done everything here you were looking to?
--
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] noblepaul commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
noblepaul commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1003210468
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
Review Comment:
why do we have a`/nodes` prefix? it's confusing
it should be a `/cluster` prefix as in `cluster/nodes`
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1005255550
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE;
+import static org.apache.solr.common.params.CollectionParams.TARGET_NODE;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
Review Comment:
This is an outdated code. The current code only accepts the targetNodeName in the request body
--
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] joshgog commented on a diff in pull request #1115: Created V2 equivalent of REPLACENODE
Posted by GitBox <gi...@apache.org>.
joshgog commented on code in PR #1115:
URL: https://github.com/apache/solr/pull/1115#discussion_r1003204389
##########
solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNodeAPI.java:
##########
@@ -0,0 +1,121 @@
+/*
+ * 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.cloud.Overseer.QUEUE_OPERATION;
+import static org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.PROPERTY_VALUE_PROP;
+import static org.apache.solr.common.cloud.ZkStateReader.TARGET_NODE_NAME_PROP;
+import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT;
+import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.Parameter;
+import io.swagger.v3.oas.annotations.media.Schema;
+import io.swagger.v3.oas.annotations.parameters.RequestBody;
+import java.util.HashMap;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.jersey.JacksonReflectMapWriter;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJerseyResponse;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for recreating replicas in one node (the source) on another node(s) (the target).
+ *
+ * <p>This API is analogous to the v1 /admin/collections?action=REPLACENODE command.
+ */
+@Path("/nodes/{nodeName}/commands/replace/{targetNodeName}")
+public class ReplaceNodeAPI extends AdminAPIBase {
+
+ @Inject
+ public ReplaceNodeAPI(
+ CoreContainer coreContainer,
+ SolrQueryRequest solrQueryRequest,
+ SolrQueryResponse solrQueryResponse) {
+ super(coreContainer, solrQueryRequest, solrQueryResponse);
+ }
+
+ @POST
+ @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
+ @PermissionName(COLL_EDIT_PERM)
+ public SolrJerseyResponse replaceNode(
+ @Parameter(description = "The name of the node to be replaced.", required = true)
+ @PathParam("nodeName")
+ String nodeName,
+ @Parameter(description = "The name of the new node.", required = true)
+ @PathParam("targetNodeName")
+ String targetNodeName,
+ @RequestBody(description = "The value of the targetNodeName", required = true)
+ ReplaceNodeRequestBody requestBody)
+ throws Exception {
+ final SolrJerseyResponse response = instantiateJerseyResponse(SolrJerseyResponse.class);
+ final CoreContainer coreContainer = fetchAndValidateZooKeeperAwareCoreContainer();
+ /** TODO Record node for log and tracing */
+ final ZkNodeProps remoteMessage = createRemoteMessage(nodeName, targetNodeName, requestBody);
+ final SolrResponse remoteResponse =
+ CollectionsHandler.submitCollectionApiCommand(
+ coreContainer,
+ coreContainer.getDistributedCollectionCommandRunner(),
+ remoteMessage,
+ CollectionParams.CollectionAction.ADDREPLICAPROP,
+ DEFAULT_COLLECTION_OP_TIMEOUT);
+ if (remoteResponse.getException() != null) {
+ throw remoteResponse.getException();
+ }
+
+ disableResponseCaching();
+ return response;
+ }
+
+ public ZkNodeProps createRemoteMessage(
+ String nodeName, String targetNodeName, ReplaceNodeRequestBody requestBody) {
+ final Map<String, Object> remoteMessage = new HashMap<>();
+ remoteMessage.put(NODE_NAME_PROP, nodeName);
+ remoteMessage.put(TARGET_NODE_NAME_PROP, targetNodeName);
Review Comment:
Not sure if I was supposed to use CollectionParams SOURCE_NODE and TARGET_NODE
--
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