You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2020/07/02 00:35:37 UTC
[incubator-pinot] branch master updated: Adding new Controller API
for and setting tag for an instance (#4952)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 47ff09b Adding new Controller API for and setting tag for an instance (#4952)
47ff09b is described below
commit 47ff09b4c5d726355a4516d2e1eb72c9705b81b9
Author: icefury71 <cs...@uber.com>
AuthorDate: Wed Jul 1 17:35:19 2020 -0700
Adding new Controller API for and setting tag for an instance (#4952)
This PR addresses the issue raised in #4082.
It introduces a new Controller API to update the instance config for an existing instance in the cluster:
- `PUT /instances/{instanceName}` with `Instance` object as the pay-load
---
.../api/resources/PinotInstanceRestletResource.java | 19 +++++++++++++++++++
.../controller/helix/ControllerRequestURLBuilder.java | 6 +-----
.../helix/core/PinotHelixResourceManager.java | 15 +++++++++++++++
.../api/PinotInstanceRestletResourceTest.java | 18 +++++++++++++++++-
.../tests/OfflineClusterIntegrationTest.java | 6 +++---
5 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
index 6f2e28f..4ba9ae2 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
@@ -31,6 +31,7 @@ import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
@@ -177,4 +178,22 @@ public class PinotInstanceRestletResource {
}
return new SuccessResponse("Successfully dropped instance");
}
+
+ @PUT
+ @Path("/instances/{instanceName}")
+ @Consumes(MediaType.APPLICATION_JSON)
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Update the specified instance", consumes = MediaType.APPLICATION_JSON, notes = "Update specified instance with given instance config")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal error")})
+ public SuccessResponse updateInstance(
+ @ApiParam(value = "Instance name", required = true, example = "Server_a.b.com_20000 | Broker_my.broker.com_30000") @PathParam("instanceName") String instanceName,
+ Instance instance) {
+ LOGGER.info("Instance update request received for instance: {}", instanceName);
+ PinotResourceManagerResponse response = pinotHelixResourceManager.updateInstance(instanceName, instance);
+ if (!response.isSuccessful()) {
+ throw new ControllerApplicationException(LOGGER, "Failure to update instance. Reason: " + response.getMessage(),
+ Response.Status.INTERNAL_SERVER_ERROR);
+ }
+ return new SuccessResponse("Instance successfully updated");
+ }
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestURLBuilder.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestURLBuilder.java
index 94b6e18..802d085 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestURLBuilder.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestURLBuilder.java
@@ -45,15 +45,11 @@ public class ControllerRequestURLBuilder {
return StringUtil.join("/", _baseUrl, "instances");
}
- public String forInstanceDelete(String instanceName) {
- return StringUtil.join("/", _baseUrl, "instances", instanceName);
- }
-
public String forInstanceState(String instanceName) {
return StringUtil.join("/", _baseUrl, "instances", instanceName, "state");
}
- public String forInstanceInformation(String instanceName) {
+ public String forInstance(String instanceName) {
return StringUtil.join("/", _baseUrl, "instances", instanceName);
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
index 7abe59a..5a0fa7c 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
@@ -362,6 +362,21 @@ public class PinotHelixResourceManager {
}
/**
+ * Update a given instance for the specified Instance ID
+ */
+ public synchronized PinotResourceManagerResponse updateInstance(String instanceIdToUpdate, Instance newInstance) {
+ if (getHelixInstanceConfig(instanceIdToUpdate) == null) {
+ return PinotResourceManagerResponse.failure("Instance " + instanceIdToUpdate + " does not exists");
+ } else {
+ InstanceConfig newConfig = InstanceUtils.toHelixInstanceConfig(newInstance);
+ if(!_helixDataAccessor.setProperty(_keyBuilder.instanceConfig(instanceIdToUpdate), newConfig)) {
+ return PinotResourceManagerResponse.failure("Unable to update instance: " + instanceIdToUpdate);
+ }
+ return PinotResourceManagerResponse.SUCCESS;
+ }
+ }
+
+ /**
* Tenant related APIs
*/
// TODO: move tenant related APIs here
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotInstanceRestletResourceTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotInstanceRestletResourceTest.java
index 02a9ad7..1919b4f 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotInstanceRestletResourceTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotInstanceRestletResourceTest.java
@@ -119,6 +119,22 @@ public class PinotInstanceRestletResourceTest extends ControllerTest {
checkInstanceInfo("Broker_2.3.4.5_1234", "Broker_2.3.4.5", 1234, new String[]{"tag_BROKER"}, null, null);
checkInstanceInfo("Server_2.3.4.5_2345", "Server_2.3.4.5", 2345, new String[]{"tag_OFFLINE", "tag_REALTIME"},
new String[]{"tag_OFFLINE", "tag_REALTIME"}, new int[]{0, 1});
+
+ // Test PUT Instance API
+ String newBrokerTag = "new-broker-tag";
+ Instance newBrokerInstance = new Instance("1.2.3.4", 1234, InstanceType.BROKER, Collections.singletonList(newBrokerTag), null);
+ String brokerInstanceId = "Broker_1.2.3.4_1234";
+ String brokerInstanceUrl = _controllerRequestURLBuilder.forInstance(brokerInstanceId);
+ sendPutRequest(brokerInstanceUrl, newBrokerInstance.toJsonString());
+
+ String newServerTag = "new-server-tag";
+ Instance newServerInstance = new Instance("1.2.3.4", 2345, InstanceType.SERVER, Collections.singletonList(newServerTag), null);
+ String serverInstanceId = "Server_1.2.3.4_2345";
+ String serverInstanceUrl = _controllerRequestURLBuilder.forInstance(serverInstanceId);
+ sendPutRequest(serverInstanceUrl, newServerInstance.toJsonString());
+
+ checkInstanceInfo(brokerInstanceId, "Broker_1.2.3.4", 1234, new String[]{newBrokerTag}, null, null);
+ checkInstanceInfo(serverInstanceId, "Server_1.2.3.4", 2345, new String[]{newServerTag}, null, null);
}
private void checkInstanceInfo(String instanceName, String hostName, int port, String[] tags, String[] pools,
@@ -128,7 +144,7 @@ public class PinotInstanceRestletResourceTest extends ControllerTest {
@Override
public Boolean apply(@Nullable Void aVoid) {
try {
- String getResponse = sendGetRequest(_controllerRequestURLBuilder.forInstanceInformation(instanceName));
+ String getResponse = sendGetRequest(_controllerRequestURLBuilder.forInstance(instanceName));
JsonNode instance = JsonUtils.stringToJsonNode(getResponse);
boolean result =
(instance.get("instanceName") != null) && (instance.get("instanceName").asText().equals(instanceName))
diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
index fa75eb2..4ed5469 100644
--- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
+++ b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
@@ -942,7 +942,7 @@ public class OfflineClusterIntegrationTest extends BaseClusterIntegrationTestSet
assertEquals(numInstances, getNumBrokers() + getNumServers() + 1);
// Try to delete a server that does not exist
- String deleteInstanceRequest = _controllerRequestURLBuilder.forInstanceDelete("potato");
+ String deleteInstanceRequest = _controllerRequestURLBuilder.forInstance("potato");
try {
sendDeleteRequest(deleteInstanceRequest);
fail("Delete should have returned a failure status (404)");
@@ -963,7 +963,7 @@ public class OfflineClusterIntegrationTest extends BaseClusterIntegrationTestSet
}
// Try to delete a live server
- deleteInstanceRequest = _controllerRequestURLBuilder.forInstanceDelete(serverName);
+ deleteInstanceRequest = _controllerRequestURLBuilder.forInstance(serverName);
try {
sendDeleteRequest(deleteInstanceRequest);
fail("Delete should have returned a failure status (409)");
@@ -991,7 +991,7 @@ public class OfflineClusterIntegrationTest extends BaseClusterIntegrationTestSet
// Try to delete a broker whose information is still live
try {
- deleteInstanceRequest = _controllerRequestURLBuilder.forInstanceDelete(brokerName);
+ deleteInstanceRequest = _controllerRequestURLBuilder.forInstance(brokerName);
sendDeleteRequest(deleteInstanceRequest);
fail("Delete should have returned a failure status (409)");
} catch (IOException e) {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org