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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
