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 2023/07/06 04:51:55 UTC
[pinot] branch master updated: Update getTenantInstances call for controller and separate POST operations on it (#10993)
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/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new a0ed20fe86 Update getTenantInstances call for controller and separate POST operations on it (#10993)
a0ed20fe86 is described below
commit a0ed20fe864a20d6d77fc005b355dc6b914b28fa
Author: Pratik Tibrewal <ti...@uber.com>
AuthorDate: Thu Jul 6 10:21:49 2023 +0530
Update getTenantInstances call for controller and separate POST operations on it (#10993)
---
.../api/resources/PinotTenantRestletResource.java | 57 ++++++++------
.../api/PinotTenantRestletResourceTest.java | 88 +++++++++++++++++++++-
.../helix/ControllerInstanceToggleTest.java | 19 -----
.../pinot/controller/helix/ControllerTest.java | 27 +++++++
.../utils/builder/ControllerRequestURLBuilder.java | 5 ++
5 files changed, 151 insertions(+), 45 deletions(-)
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTenantRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTenantRestletResource.java
index d1c89d206a..7b14b6fb0f 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTenantRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTenantRestletResource.java
@@ -203,21 +203,37 @@ public class PinotTenantRestletResource {
@GET
@Path("/tenants/{tenantName}")
@Produces(MediaType.APPLICATION_JSON)
- @ApiOperation(value = "List instance for a tenant, or enable/disable/drop a tenant")
+ @ApiOperation(value = "List instance for a tenant")
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"),
@ApiResponse(code = 500, message = "Error reading tenants list")
})
- public String listInstanceOrToggleTenantState(
+ public String listInstance(
@ApiParam(value = "Tenant name", required = true) @PathParam("tenantName") String tenantName,
@ApiParam(value = "Tenant type (server|broker)") @QueryParam("type") String tenantType,
- @ApiParam(value = "Table type (offline|realtime)") @QueryParam("tableType") String tableType,
- @ApiParam(value = "state") @QueryParam("state") String stateStr)
- throws Exception {
- if (stateStr == null) {
- return listInstancesForTenant(tenantName, tenantType, tableType);
- } else {
+ @ApiParam(value = "Table type (offline|realtime)") @QueryParam("tableType") String tableType) {
+ return listInstancesForTenant(tenantName, tenantType, tableType);
+ }
+
+ @POST
+ @Path("/tenants/{tenantName}")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "enable/disable a tenant")
+ @ApiResponses(value = {
+ @ApiResponse(code = 200, message = "Success"),
+ @ApiResponse(code = 500, message = "Error applying state to tenant")
+ })
+ public SuccessResponse enableOrDisableTenant(
+ @ApiParam(value = "Tenant name", required = true) @PathParam("tenantName") String tenantName,
+ @ApiParam(value = "Tenant type (server|broker)") @QueryParam("type") String tenantType,
+ @ApiParam(value = "state (enable|disable)") @QueryParam("state") String stateStr) {
+ if (stateStr.equalsIgnoreCase(String.valueOf(StateType.ENABLE))
+ || stateStr.equalsIgnoreCase(String.valueOf(StateType.DISABLE))) {
return toggleTenantState(tenantName, stateStr, tenantType);
+ } else {
+ throw new ControllerApplicationException(LOGGER,
+ "Error: State mentioned " + stateStr + " is wrong. Valid States: Enable, Disable",
+ Response.Status.BAD_REQUEST);
}
}
@@ -260,7 +276,7 @@ public class PinotTenantRestletResource {
return resourceGetRet.toString();
}
- private String toggleTenantState(String tenantName, String stateStr, @Nullable String tenantType) {
+ private SuccessResponse toggleTenantState(String tenantName, String stateStr, @Nullable String tenantType) {
Set<String> serverInstances = new HashSet<>();
Set<String> brokerInstances = new HashSet<>();
ObjectNode instanceResult = JsonUtils.newObjectNode();
@@ -276,27 +292,17 @@ public class PinotTenantRestletResource {
Set<String> allInstances = new HashSet<String>(serverInstances);
allInstances.addAll(brokerInstances);
- if (StateType.DROP.name().equalsIgnoreCase(stateStr)) {
- if (!allInstances.isEmpty()) {
- throw new ControllerApplicationException(LOGGER,
- "Error: Tenant " + tenantName + " has live instances, cannot be dropped.", Response.Status.BAD_REQUEST);
+ if (StateType.DISABLE.name().equalsIgnoreCase(stateStr)) {
+ for (String instance : allInstances) {
+ instanceResult.put(instance, JsonUtils.objectToJsonNode(_pinotHelixResourceManager.disableInstance(instance)));
}
- _pinotHelixResourceManager.deleteBrokerTenantFor(tenantName);
- _pinotHelixResourceManager.deleteOfflineServerTenantFor(tenantName);
- _pinotHelixResourceManager.deleteRealtimeServerTenantFor(tenantName);
- return new SuccessResponse("Dropped tenant " + tenantName + " successfully.").toString();
}
-
- boolean enable = StateType.ENABLE.name().equalsIgnoreCase(stateStr) ? true : false;
- for (String instance : allInstances) {
- if (enable) {
+ if (StateType.ENABLE.name().equalsIgnoreCase(stateStr)) {
+ for (String instance : allInstances) {
instanceResult.put(instance, JsonUtils.objectToJsonNode(_pinotHelixResourceManager.enableInstance(instance)));
- } else {
- instanceResult.put(instance, JsonUtils.objectToJsonNode(_pinotHelixResourceManager.disableInstance(instance)));
}
}
-
- return null;
+ return new SuccessResponse("Changed state of tenant " + tenantName + " to " + stateStr + " successfully.");
}
private String listInstancesForTenant(String tenantName, String tenantType, String tableTypeString) {
@@ -401,6 +407,7 @@ public class PinotTenantRestletResource {
// CHANGE-ALERT: This is not backward compatible. We've changed this API from GET to POST because:
// 1. That is correct
// 2. with GET, we need to write our own routing logic to avoid conflict since this is same as the API above
+ @Deprecated
@POST
@Path("/tenants/{tenantName}/metadata")
@Authenticate(AccessType.UPDATE)
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTenantRestletResourceTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTenantRestletResourceTest.java
index d34c5e6806..a55e223551 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTenantRestletResourceTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTenantRestletResourceTest.java
@@ -19,24 +19,33 @@
package org.apache.pinot.controller.api;
import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
import org.apache.helix.store.zk.ZkHelixPropertyStore;
import org.apache.helix.zookeeper.datamodel.ZNRecord;
import org.apache.pinot.common.utils.config.TagNameUtils;
import org.apache.pinot.controller.helix.ControllerTest;
+import org.apache.pinot.controller.utils.SegmentMetadataMockUtils;
+import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
-public class PinotTenantRestletResourceTest {
+public class PinotTenantRestletResourceTest extends ControllerTest {
private static final ControllerTest TEST_INSTANCE = ControllerTest.getInstance();
private static final String TABLE_NAME = "restletTable_OFFLINE";
+ private static final String RAW_TABLE_NAME = "toggleTable";
+ private static final String OFFLINE_TABLE_NAME = TableNameBuilder.OFFLINE.tableNameWithType(RAW_TABLE_NAME);
+
@BeforeClass
public void setUp()
@@ -82,6 +91,83 @@ public class PinotTenantRestletResourceTest {
assertEquals(tables.size(), 0);
}
+ @Test
+ public void testListInstance()
+ throws Exception {
+ String listInstancesUrl =
+ TEST_INSTANCE.getControllerRequestURLBuilder().forTenantGet(TagNameUtils.DEFAULT_TENANT_NAME);
+ JsonNode instanceList = JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listInstancesUrl));
+ assertEquals(instanceList.get("ServerInstances").size(), DEFAULT_NUM_SERVER_INSTANCES);
+ assertEquals(instanceList.get("BrokerInstances").size(), DEFAULT_NUM_BROKER_INSTANCES);
+ }
+
+ @Test
+ public void testToggleTenantState()
+ throws Exception {
+ // Create an offline table
+ TableConfig tableConfig =
+ new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).setNumReplicas(DEFAULT_MIN_NUM_REPLICAS)
+ .build();
+ sendPostRequest(TEST_INSTANCE.getControllerRequestURLBuilder().forTableCreate(), tableConfig.toJsonString());
+ assertEquals(TEST_INSTANCE.getHelixAdmin()
+ .getResourceIdealState(TEST_INSTANCE.getHelixClusterName(), CommonConstants.Helix.BROKER_RESOURCE_INSTANCE)
+ .getInstanceSet(OFFLINE_TABLE_NAME).size(), DEFAULT_NUM_BROKER_INSTANCES);
+
+ // Add segments
+ for (int i = 0; i < DEFAULT_NUM_SERVER_INSTANCES; i++) {
+ TEST_INSTANCE.getHelixResourceManager()
+ .addNewSegment(OFFLINE_TABLE_NAME, SegmentMetadataMockUtils.mockSegmentMetadata(RAW_TABLE_NAME),
+ "downloadUrl");
+ assertEquals(TEST_INSTANCE.getHelixAdmin()
+ .getResourceIdealState(TEST_INSTANCE.getHelixClusterName(), OFFLINE_TABLE_NAME).getNumPartitions(), i + 1);
+ }
+
+ // Disable server instances
+ String disableServerInstanceUrl =
+ TEST_INSTANCE.getControllerRequestURLBuilder().forTenantInstancesToggle(TagNameUtils.DEFAULT_TENANT_NAME,
+ "server", "disable");
+ JsonUtils.stringToJsonNode(ControllerTest.sendPostRequest(disableServerInstanceUrl));
+ checkNumOnlineInstancesFromExternalView(OFFLINE_TABLE_NAME, 0);
+
+ // Enable server instances
+ String enableServerInstanceUrl =
+ TEST_INSTANCE.getControllerRequestURLBuilder().forTenantInstancesToggle(TagNameUtils.DEFAULT_TENANT_NAME,
+ "server", "enable");
+ JsonUtils.stringToJsonNode(ControllerTest.sendPostRequest(enableServerInstanceUrl));
+ checkNumOnlineInstancesFromExternalView(OFFLINE_TABLE_NAME, DEFAULT_NUM_SERVER_INSTANCES);
+
+ // Disable broker instances
+ String disableBrokerInstanceUrl =
+ TEST_INSTANCE.getControllerRequestURLBuilder().forTenantInstancesToggle(TagNameUtils.DEFAULT_TENANT_NAME,
+ "broker", "disable");
+ JsonUtils.stringToJsonNode(ControllerTest.sendPostRequest(disableBrokerInstanceUrl));
+ checkNumOnlineInstancesFromExternalView(CommonConstants.Helix.BROKER_RESOURCE_INSTANCE, 0);
+
+ // Enable broker instances
+ String enableBrokerInstanceUrl =
+ TEST_INSTANCE.getControllerRequestURLBuilder().forTenantInstancesToggle(TagNameUtils.DEFAULT_TENANT_NAME,
+ "broker", "enable");
+ JsonUtils.stringToJsonNode(ControllerTest.sendPostRequest(enableBrokerInstanceUrl));
+ checkNumOnlineInstancesFromExternalView(CommonConstants.Helix.BROKER_RESOURCE_INSTANCE,
+ DEFAULT_NUM_BROKER_INSTANCES);
+
+ // Delete table
+ sendDeleteRequest(TEST_INSTANCE.getControllerRequestURLBuilder().forTableDelete(RAW_TABLE_NAME));
+
+ // Check exception in case of enum mismatch of State
+ try {
+ String mismatchStateBrokerInstanceUrl =
+ TEST_INSTANCE.getControllerRequestURLBuilder().forTenantInstancesToggle(TagNameUtils.DEFAULT_TENANT_NAME,
+ "broker", "random");
+ sendPostRequest(mismatchStateBrokerInstanceUrl);
+ fail("Passing invalid state to tenant toggle state does not fail.");
+ } catch (IOException e) {
+ // Expected 500 Bad Request
+ assertTrue(e.getMessage().contains("Error: State mentioned random is wrong. "
+ + "Valid States: Enable, Disable"));
+ }
+ }
+
@AfterClass
public void tearDown() {
TEST_INSTANCE.cleanup();
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerInstanceToggleTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerInstanceToggleTest.java
index 746966ec47..9528e2dbbf 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerInstanceToggleTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerInstanceToggleTest.java
@@ -19,10 +19,7 @@
package org.apache.pinot.controller.helix;
import java.io.IOException;
-import java.util.Set;
-import org.apache.helix.model.ExternalView;
import org.apache.pinot.common.utils.config.TagNameUtils;
-import org.apache.pinot.common.utils.helix.HelixHelper;
import org.apache.pinot.controller.utils.SegmentMetadataMockUtils;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
@@ -35,7 +32,6 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.fail;
public class ControllerInstanceToggleTest extends ControllerTest {
@@ -129,21 +125,6 @@ public class ControllerInstanceToggleTest extends ControllerTest {
}, TIMEOUT_MS, "Failed to toggle instance state: '" + state + "' for instance: " + instanceName);
}
- private void checkNumOnlineInstancesFromExternalView(String resourceName, int expectedNumOnlineInstances)
- throws InterruptedException {
- long endTime = System.currentTimeMillis() + TIMEOUT_MS;
- while (System.currentTimeMillis() < endTime) {
- ExternalView resourceExternalView = DEFAULT_INSTANCE.getHelixAdmin()
- .getResourceExternalView(DEFAULT_INSTANCE.getHelixClusterName(), resourceName);
- Set<String> instanceSet = HelixHelper.getOnlineInstanceFromExternalView(resourceExternalView);
- if (instanceSet.size() == expectedNumOnlineInstances) {
- return;
- }
- Thread.sleep(100L);
- }
- fail("Failed to reach " + expectedNumOnlineInstances + " online instances for resource: " + resourceName);
- }
-
@AfterClass
public void tearDown() {
DEFAULT_INSTANCE.cleanup();
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 63f575293d..f6c7d29ecf 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -43,6 +43,7 @@ import org.apache.helix.HelixManagerFactory;
import org.apache.helix.InstanceType;
import org.apache.helix.NotificationContext;
import org.apache.helix.model.ClusterConfig;
+import org.apache.helix.model.ExternalView;
import org.apache.helix.model.HelixConfigScope;
import org.apache.helix.model.Message;
import org.apache.helix.model.ResourceConfig;
@@ -58,6 +59,7 @@ import org.apache.pinot.common.exception.HttpErrorStatusException;
import org.apache.pinot.common.utils.SimpleHttpResponse;
import org.apache.pinot.common.utils.ZkStarter;
import org.apache.pinot.common.utils.config.TagNameUtils;
+import org.apache.pinot.common.utils.helix.HelixHelper;
import org.apache.pinot.common.utils.http.HttpClient;
import org.apache.pinot.controller.BaseControllerStarter;
import org.apache.pinot.controller.ControllerConf;
@@ -86,6 +88,7 @@ import static org.apache.pinot.spi.utils.CommonConstants.Helix.UNTAGGED_SERVER_I
import static org.apache.pinot.spi.utils.CommonConstants.Server.DEFAULT_ADMIN_API_PORT;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.fail;
public class ControllerTest {
@@ -105,6 +108,8 @@ public class ControllerTest {
// NOTE: To add HLC realtime table, number of Server instances must be multiple of replicas
public static final int DEFAULT_NUM_SERVER_INSTANCES = 4;
+ public static final long TIMEOUT_MS = 10_000L;
+
/**
* default static instance used to access all wrapped static instances.
*/
@@ -771,6 +776,10 @@ public class ControllerTest {
return IOUtils.toString(new URL(urlString).openStream());
}
+ public static String sendPostRequest(String urlString)
+ throws IOException {
+ return sendPostRequest(urlString, null);
+ }
public static String sendPostRequest(String urlString, String payload)
throws IOException {
return sendPostRequest(urlString, payload, Collections.emptyMap());
@@ -965,6 +974,24 @@ public class ControllerTest {
stopZk();
}
+ /**
+ * Checks if the number of online instances for a given resource matches the expected num of instances or not.
+ */
+ public void checkNumOnlineInstancesFromExternalView(String resourceName, int expectedNumOnlineInstances)
+ throws InterruptedException {
+ long endTime = System.currentTimeMillis() + TIMEOUT_MS;
+ while (System.currentTimeMillis() < endTime) {
+ ExternalView resourceExternalView = DEFAULT_INSTANCE.getHelixAdmin()
+ .getResourceExternalView(DEFAULT_INSTANCE.getHelixClusterName(), resourceName);
+ Set<String> instanceSet = HelixHelper.getOnlineInstanceFromExternalView(resourceExternalView);
+ if (instanceSet.size() == expectedNumOnlineInstances) {
+ return;
+ }
+ Thread.sleep(100L);
+ }
+ fail("Failed to reach " + expectedNumOnlineInstances + " online instances for resource: " + resourceName);
+ }
+
/**
* Make sure shared state is setup and valid before each test case class is run.
*/
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/ControllerRequestURLBuilder.java b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/ControllerRequestURLBuilder.java
index 39a873e683..83047b7903 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/ControllerRequestURLBuilder.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/ControllerRequestURLBuilder.java
@@ -163,6 +163,11 @@ public class ControllerRequestURLBuilder {
return StringUtil.join("/", _baseUrl, "brokers", "tables", "?state=" + state);
}
+ public String forTenantInstancesToggle(String tenant, String tenantType, String state) {
+ return StringUtil.join("/", _baseUrl, "tenants", tenant)
+ + "?type=" + tenantType + "&state=" + state;
+ }
+
public String forLiveBrokerTablesGet() {
return StringUtil.join("/", _baseUrl, "tables", "livebrokers");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org