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 6bce5faa54 Update the pinot tenants tables api to support returning
broker tagged tables (#11184)
6bce5faa54 is described below
commit 6bce5faa5480e3f5943157c4f398cbd9a360745b
Author: Johan Adami <4760722+jadam...@users.noreply.github.com>
AuthorDate: Tue Aug 1 18:49:06 2023 -0400
Update the pinot tenants tables api to support returning broker tagged
tables (#11184)
---
.../api/resources/PinotTenantRestletResource.java | 39 +++++++++++--
.../controller/helix/ControllerRequestClient.java | 10 ++++
.../api/PinotTenantRestletResourceTest.java | 66 +++++++++++++++++++---
.../pinot/controller/helix/ControllerTest.java | 10 ++++
.../utils/builder/ControllerRequestURLBuilder.java | 4 ++
5 files changed, 118 insertions(+), 11 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 59180c0805..70a24e15e4 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
@@ -249,23 +249,34 @@ public class PinotTenantRestletResource {
* This method expects a tenant name and will return a list of tables tagged
on that tenant. It assumes that the
* tagname is for server tenants only.
* @param tenantName
+ * @param tenantType
* @return
*/
@GET
@Path("/tenants/{tenantName}/tables")
@Authorize(targetType = TargetType.CLUSTER, action =
Actions.Cluster.GET_TENANT)
@Produces(MediaType.APPLICATION_JSON)
- @ApiOperation(value = "List tables on a a server tenant")
+ @ApiOperation(value = "List tables on a server or broker tenant")
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"),
@ApiResponse(code = 500, message = "Error reading list")
})
public String getTablesOnTenant(
- @ApiParam(value = "Tenant name", required = true)
@PathParam("tenantName") String tenantName) {
- return getTablesServedFromTenant(tenantName);
+ @ApiParam(value = "Tenant name", required = true)
@PathParam("tenantName") String tenantName,
+ @ApiParam(value = "Tenant type (server|broker)",
+ required = false, allowableValues = "BROKER, SERVER", defaultValue =
"SERVER")
+ @QueryParam("type") String tenantType) {
+ if (tenantType == null || tenantType.isEmpty() ||
tenantType.equalsIgnoreCase("server")) {
+ return getTablesServedFromServerTenant(tenantName);
+ } else if (tenantType.equalsIgnoreCase("broker")) {
+ return getTablesServedFromBrokerTenant(tenantName);
+ } else {
+ throw new ControllerApplicationException(LOGGER, "Invalid tenant type: "
+ tenantType,
+ Response.Status.BAD_REQUEST);
+ }
}
- private String getTablesServedFromTenant(String tenantName) {
+ private String getTablesServedFromServerTenant(String tenantName) {
Set<String> tables = new HashSet<>();
ObjectNode resourceGetRet = JsonUtils.newObjectNode();
@@ -285,6 +296,26 @@ public class PinotTenantRestletResource {
return resourceGetRet.toString();
}
+ private String getTablesServedFromBrokerTenant(String tenantName) {
+ Set<String> tables = new HashSet<>();
+ ObjectNode resourceGetRet = JsonUtils.newObjectNode();
+
+ for (String table : _pinotHelixResourceManager.getAllTables()) {
+ TableConfig tableConfig =
_pinotHelixResourceManager.getTableConfig(table);
+ if (tableConfig == null) {
+ LOGGER.error("Unable to retrieve table config for table: {}", table);
+ continue;
+ }
+ String tableConfigTenant = tableConfig.getTenantConfig().getBroker();
+ if (tenantName.equals(tableConfigTenant)) {
+ tables.add(table);
+ }
+ }
+
+ resourceGetRet.set(TABLES, JsonUtils.objectToJsonNode(tables));
+ return resourceGetRet.toString();
+ }
+
private SuccessResponse toggleTenantState(String tenantName, String
stateStr, @Nullable String tenantType) {
Set<String> serverInstances = new HashSet<>();
Set<String> brokerInstances = new HashSet<>();
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestClient.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestClient.java
index e6aa83dea1..3ec7fc3642 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestClient.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/ControllerRequestClient.java
@@ -291,6 +291,16 @@ public class ControllerRequestClient {
}
}
+ public void deleteBrokerTenant(String tenantName)
+ throws IOException {
+ try {
+ HttpClient.wrapAndThrowHttpException(_httpClient.sendDeleteRequest(new
URL(
+
_controllerRequestURLBuilder.forBrokerTenantDelete(tenantName)).toURI()));
+ } catch (HttpErrorStatusException | URISyntaxException e) {
+ throw new IOException(e);
+ }
+ }
+
public void createServerTenant(String tenantName, int numOfflineServers, int
numRealtimeServers)
throws IOException {
try {
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 a55e223551..289f070a14 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
@@ -62,22 +62,63 @@ public class PinotTenantRestletResourceTest extends
ControllerTest {
JsonNode tableList =
JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listTablesUrl));
assertEquals(tableList.get("tables").size(), 0);
+ // Add some non default tag broker instances, UNTAGGED_BROKER_INSTANCE
+ String brokerTag2 = "brokerTag2";
+ TEST_INSTANCE.addFakeBrokerInstanceToAutoJoinHelixCluster("broker_999",
false);
+ TEST_INSTANCE.addFakeBrokerInstanceToAutoJoinHelixCluster("broker_1000",
false);
+ TEST_INSTANCE.updateBrokerTenant("brokerTag2", 2);
+
// Add a table
ControllerTest.sendPostRequest(TEST_INSTANCE.getControllerRequestURLBuilder().forTableCreate(),
new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).build().toJsonString());
- // There should be 1 table on the tenant
+ // Add a second table with a different broker tag
+ String table2 = "restletTable2_OFFLINE";
+
ControllerTest.sendPostRequest(TEST_INSTANCE.getControllerRequestURLBuilder().forTableCreate(),
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(table2).setBrokerTenant(
+ brokerTag2)
+ .build().toJsonString());
+
+ // There should be 2 tables on the tenant when querying default Tenant for
servers w/o specifying ?type=server
tableList =
JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listTablesUrl));
JsonNode tables = tableList.get("tables");
- assertEquals(tables.size(), 1);
+ assertEquals(tables.size(), 2);
+
+ // There should be 2 tables even when specifying ?type=server as that is
the default
+ listTablesUrl =
+
TEST_INSTANCE.getControllerRequestURLBuilder().forTablesFromTenant(TagNameUtils.DEFAULT_TENANT_NAME,
+ "server");
+ tableList =
JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listTablesUrl));
+ tables = tableList.get("tables");
- // Check to make sure that test table exists.
- boolean found = false;
- for (int i = 0; !found && i < tables.size(); i++) {
- found = tables.get(i).asText().equals(TABLE_NAME);
+ // Check to make sure that test tables exists.
+ boolean found1 = false;
+ boolean found2 = false;
+ for (int i = 0; i < tables.size(); i++) {
+ found1 = found1 || tables.get(i).asText().equals(TABLE_NAME);
+ found2 = found2 || tables.get(i).asText().equals(table2);
}
- assertTrue(found);
+ // There should be only 1 table when specifying ?type=broker for the
default tenant
+ listTablesUrl =
+
TEST_INSTANCE.getControllerRequestURLBuilder().forTablesFromTenant(TagNameUtils.DEFAULT_TENANT_NAME,
+ "broker");
+ tableList =
JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listTablesUrl));
+ tables = tableList.get("tables");
+ assertEquals(tables.size(), 1);
+
+ String defaultTenantTable = tables.get(0).asText();
+
+ // There should be only 1 table when specifying ?type=broker for the
broker_untagged tenant
+ listTablesUrl =
+
TEST_INSTANCE.getControllerRequestURLBuilder().forTablesFromTenant(brokerTag2,
+ "broker");
+ tableList =
JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listTablesUrl));
+ tables = tableList.get("tables");
+ assertEquals(tables.size(), 1);
+
+ // This second table should not be the same as the one from the default
tenant
+ assertTrue(!defaultTenantTable.equals(tables.get(0).asText()));
// reset the ZK node to simulate corruption
ZkHelixPropertyStore<ZNRecord> propertyStore =
TEST_INSTANCE.getPropertyStore();
@@ -85,10 +126,21 @@ public class PinotTenantRestletResourceTest extends
ControllerTest {
ZNRecord znRecord = propertyStore.get(zkPath, null, 0);
propertyStore.set(zkPath, new ZNRecord(znRecord.getId()), 1);
+ // corrupt the other one also
+ zkPath = "/CONFIGS/TABLE/" + table2;
+ znRecord = propertyStore.get(zkPath, null, 0);
+ propertyStore.set(zkPath, new ZNRecord(znRecord.getId()), 1);
+
// Now there should be no tables
tableList =
JsonUtils.stringToJsonNode(ControllerTest.sendGetRequest(listTablesUrl));
tables = tableList.get("tables");
assertEquals(tables.size(), 0);
+
+ // remove the additional, non-default table and broker instances
+ TEST_INSTANCE.dropOfflineTable(table2);
+ TEST_INSTANCE.stopAndDropFakeInstance("broker_999");
+ TEST_INSTANCE.stopAndDropFakeInstance("broker_1000");
+ TEST_INSTANCE.deleteBrokerTenant(brokerTag2);
}
@Test
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 43aef8af8b..ef097e99ae 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
@@ -597,6 +597,11 @@ public class ControllerTest {
}
}
+ public void stopAndDropFakeInstance(String instanceId) {
+ stopFakeInstance(instanceId);
+ _helixResourceManager.dropInstance(instanceId);
+ }
+
public static Schema createDummySchema(String tableName) {
Schema schema = new Schema();
schema.setSchemaName(tableName);
@@ -735,6 +740,11 @@ public class ControllerTest {
getControllerRequestClient().updateBrokerTenant(tenantName, numBrokers);
}
+ public void deleteBrokerTenant(String tenantName)
+ throws IOException {
+ getControllerRequestClient().deleteBrokerTenant(tenantName);
+ }
+
public void createServerTenant(String tenantName, int numOfflineServers, int
numRealtimeServers)
throws IOException {
getControllerRequestClient().createServerTenant(tenantName,
numOfflineServers, numRealtimeServers);
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 fe04bc3e19..3a57293288 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
@@ -85,6 +85,10 @@ public class ControllerRequestURLBuilder {
return StringUtil.join("/", _baseUrl, "tenants", tenantName, "tables");
}
+ public String forTablesFromTenant(String tenantName, String componentType) {
+ return StringUtil.join("/", _baseUrl, "tenants", tenantName, "tables") +
"?type=" + componentType;
+ }
+
// V2 API started
public String forTenantCreate() {
return StringUtil.join("/", _baseUrl, "tenants");
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
