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 2019/08/01 19:54:19 UTC
[incubator-pinot] branch master updated: Clean up tenant config
validation to correctly handle tag override (#4488)
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 6cf8c60 Clean up tenant config validation to correctly handle tag override (#4488)
6cf8c60 is described below
commit 6cf8c60a982cfc3aa47d08ae90d2a6a9d9df712d
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Thu Aug 1 12:54:14 2019 -0700
Clean up tenant config validation to correctly handle tag override (#4488)
- Simplify the validation logic
- Fetch server tag from TagConfig to handle tag override properly
---
.../apache/pinot/common/config/TagNameUtils.java | 10 +-
.../resources/PinotTableConfigRestletResource.java | 35 +++--
.../api/resources/PinotTableRestletResource.java | 24 +--
.../helix/core/PinotHelixResourceManager.java | 125 ++++++---------
.../helix/core/PinotHelixResourceManagerTest.java | 170 +++++++--------------
.../ReplicaGroupRebalanceStrategyTest.java | 12 +-
.../sharding/SegmentAssignmentStrategyTest.java | 6 +-
7 files changed, 132 insertions(+), 250 deletions(-)
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
index a22e4fa..1702f53 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/config/TagNameUtils.java
@@ -23,6 +23,9 @@ import org.apache.pinot.common.utils.TenantRole;
public class TagNameUtils {
+ private TagNameUtils() {
+ }
+
public final static String DEFAULT_TENANT_NAME = "DefaultTenant";
private static String buildRealtimeTagFromTenantName(String tenantName) {
@@ -53,13 +56,6 @@ public class TagNameUtils {
return tagName.endsWith(TenantRole.BROKER.toString());
}
- public static String getTagFromTenantAndServerType(String tenantName, ServerType type) {
- if (type == ServerType.OFFLINE) {
- return getOfflineTagForTenant(tenantName);
- }
- return getRealtimeTagForTenant(tenantName);
- }
-
public static String getRealtimeTagForTenant(String tenantName) {
if (tenantName == null) {
return TagNameUtils.getRealtimeTagForTenant(DEFAULT_TENANT_NAME);
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableConfigRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableConfigRestletResource.java
index 5717170..af5e699 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableConfigRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableConfigRestletResource.java
@@ -150,41 +150,40 @@ public class PinotTableConfigRestletResource {
.type(MediaType.TEXT_PLAIN_TYPE).build();
}
- if (config.getSchema() != null) {
- _resourceManager.addOrUpdateSchema(config.getSchema());
+ Schema schema = config.getSchema();
+ if (schema != null) {
+ _resourceManager.addOrUpdateSchema(schema);
}
- if (config.getOfflineTableConfig() != null) {
- if (_resourceManager.getAllTables().contains(config.getOfflineTableConfig().getTableName())) {
+ TableConfig offlineTableConfig = config.getOfflineTableConfig();
+ if (offlineTableConfig != null) {
+ String offlineTableName = offlineTableConfig.getTableName();
+ if (_resourceManager.hasTable(offlineTableName)) {
try {
- _resourceManager
- .setExistingTableConfig(config.getOfflineTableConfig(), config.getOfflineTableConfig().getTableName(),
- CommonConstants.Helix.TableType.OFFLINE);
+ _resourceManager.setExistingTableConfig(offlineTableConfig);
} catch (IOException e) {
- LOGGER.warn("Failed to update the offline table configuration for table {}", e,
- config.getOfflineTableConfig().getTableName());
+ LOGGER.warn("Failed to update the table config for table: {}", offlineTableName, e);
return Response.serverError().entity("Failed to update the offline table configuration")
.type(MediaType.TEXT_PLAIN_TYPE).build();
}
} else {
- _resourceManager.addTable(config.getOfflineTableConfig());
+ _resourceManager.addTable(offlineTableConfig);
}
}
- if (config.getRealtimeTableConfig() != null) {
- if (_resourceManager.getAllTables().contains(config.getRealtimeTableConfig().getTableName())) {
+ TableConfig realtimeTableConfig = config.getRealtimeTableConfig();
+ if (realtimeTableConfig != null) {
+ String realtimeTableName = realtimeTableConfig.getTableName();
+ if (_resourceManager.hasTable(realtimeTableName)) {
try {
- _resourceManager
- .setExistingTableConfig(config.getRealtimeTableConfig(), config.getRealtimeTableConfig().getTableName(),
- CommonConstants.Helix.TableType.REALTIME);
+ _resourceManager.setExistingTableConfig(realtimeTableConfig);
} catch (IOException e) {
- LOGGER.warn("Failed to update the realtime table configuration for table {}", e,
- config.getRealtimeTableConfig().getTableName());
+ LOGGER.warn("Failed to update the table config for table: {}", realtimeTableName, e);
return Response.serverError().entity("Failed to update the realtime table configuration")
.type(MediaType.TEXT_PLAIN_TYPE).build();
}
} else {
- _resourceManager.addTable(config.getRealtimeTableConfig());
+ _resourceManager.addTable(realtimeTableConfig);
}
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 74919e9..f22a1f2 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -286,30 +286,22 @@ public class PinotTableRestletResource {
}
try {
- CommonConstants.Helix.TableType tableType = tableConfig.getTableType();
- String configTableName = tableConfig.getTableName();
- String tableNameWithType = TableNameBuilder.forType(tableType).tableNameWithType(tableName);
- if (!configTableName.equals(tableNameWithType)) {
+ String tableNameWithType = tableConfig.getTableName();
+ if (!TableNameBuilder.forType(tableConfig.getTableType()).tableNameWithType(tableName)
+ .equals(tableNameWithType)) {
throw new ControllerApplicationException(LOGGER,
- "Request table " + tableNameWithType + " does not match table name in the body " + configTableName,
+ "Request table " + tableName + " does not match table name in the body " + tableNameWithType,
Response.Status.BAD_REQUEST);
}
- if (tableType == CommonConstants.Helix.TableType.OFFLINE) {
- if (!_pinotHelixResourceManager.hasOfflineTable(tableName)) {
- throw new ControllerApplicationException(LOGGER, "Table " + tableName + " does not exist",
- Response.Status.BAD_REQUEST);
- }
- } else {
- if (!_pinotHelixResourceManager.hasRealtimeTable(tableName)) {
- throw new ControllerApplicationException(LOGGER, "Table " + tableName + " does not exist",
- Response.Status.NOT_FOUND);
- }
+ if (!_pinotHelixResourceManager.hasTable(tableNameWithType)) {
+ throw new ControllerApplicationException(LOGGER, "Table " + tableNameWithType + " does not exist",
+ Response.Status.NOT_FOUND);
}
ensureMinReplicas(tableConfig);
verifyTableConfigs(tableConfig);
- _pinotHelixResourceManager.updateTableConfig(tableConfig, tableNameWithType, tableType);
+ _pinotHelixResourceManager.updateTableConfig(tableConfig);
} catch (PinotHelixResourceManager.InvalidTableConfigException e) {
String errStr = String.format("Failed to update configuration for %s due to: %s", tableName, e.getMessage());
_controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_TABLE_UPDATE_ERROR, 1L);
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 63c2b2f..b6d4b03 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
@@ -35,6 +35,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -71,7 +72,6 @@ import org.apache.pinot.common.config.TableConfig;
import org.apache.pinot.common.config.TableCustomConfig;
import org.apache.pinot.common.config.TableNameBuilder;
import org.apache.pinot.common.config.TagNameUtils;
-import org.apache.pinot.common.config.TagOverrideConfig;
import org.apache.pinot.common.config.Tenant;
import org.apache.pinot.common.config.TenantConfig;
import org.apache.pinot.common.data.Schema;
@@ -797,19 +797,6 @@ public class PinotHelixResourceManager {
return PinotResourceManagerResponse.failure(message);
}
- public boolean isTenantExisted(String tenantName) {
- if (!HelixHelper.getInstancesWithTag(_helixZkManager, TagNameUtils.getBrokerTagForTenant(tenantName)).isEmpty()) {
- return true;
- }
- if (!HelixHelper.getInstancesWithTag(_helixZkManager, TagNameUtils.getOfflineTagForTenant(tenantName)).isEmpty()) {
- return true;
- }
- if (!HelixHelper.getInstancesWithTag(_helixZkManager, TagNameUtils.getRealtimeTagForTenant(tenantName)).isEmpty()) {
- return true;
- }
- return false;
- }
-
public boolean isBrokerTenantDeletable(String tenantName) {
String brokerTag = TagNameUtils.getBrokerTagForTenant(tenantName);
Set<String> taggedInstances = new HashSet<>(HelixHelper.getInstancesWithTag(_helixZkManager, brokerTag));
@@ -1055,17 +1042,16 @@ public class PinotHelixResourceManager {
*/
public void addTable(@Nonnull TableConfig tableConfig)
throws IOException {
- final String tableNameWithType = tableConfig.getTableName();
- TableType tableType = tableConfig.getTableType();
-
if (isSingleTenantCluster()) {
TenantConfig tenantConfig = new TenantConfig();
tenantConfig.setBroker(TagNameUtils.DEFAULT_TENANT_NAME);
tenantConfig.setServer(TagNameUtils.DEFAULT_TENANT_NAME);
tableConfig.setTenantConfig(tenantConfig);
}
- validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ validateTableTenantConfig(tableConfig);
+ String tableNameWithType = tableConfig.getTableName();
+ TableType tableType = tableConfig.getTableType();
SegmentsValidationAndRetentionConfig segmentsConfig = tableConfig.getValidationConfig();
switch (tableType) {
case OFFLINE:
@@ -1141,55 +1127,34 @@ public class PinotHelixResourceManager {
* Validates the tenant config for the table
*/
@VisibleForTesting
- protected void validateTableTenantConfig(TableConfig tableConfig, String tableNameWithType, TableType tableType) {
- if (tableConfig == null) {
- throw new InvalidTableConfigException("Table config is null for table: " + tableNameWithType);
- }
+ void validateTableTenantConfig(TableConfig tableConfig) {
+ String tableNameWithType = tableConfig.getTableName();
TenantConfig tenantConfig = tableConfig.getTenantConfig();
- if (tenantConfig == null || tenantConfig.getBroker() == null || tenantConfig.getServer() == null) {
- throw new InvalidTableConfigException("Tenant is not configured for table: " + tableNameWithType);
- }
+
// Check if tenant exists before creating the table
- String brokerTenantName = TagNameUtils.getBrokerTagForTenant(tenantConfig.getBroker());
- List<String> brokersForTenant = getInstancesWithTag(brokerTenantName);
- if (brokersForTenant.isEmpty()) {
- throw new InvalidTableConfigException(
- "Broker tenant: " + brokerTenantName + " does not exist for table: " + tableNameWithType);
- }
- String serverTenantName =
- TagNameUtils.getTagFromTenantAndServerType(tenantConfig.getServer(), tableType.getServerType());
- if (getInstancesWithTag(serverTenantName).isEmpty()) {
- throw new InvalidTableConfigException(
- "Server tenant: " + serverTenantName + " does not exist for table: " + tableNameWithType);
- }
- TagOverrideConfig tagOverrideConfig = tenantConfig.getTagOverrideConfig();
- if (tagOverrideConfig != null) {
- String realtimeConsumingTag = tagOverrideConfig.getRealtimeConsuming();
- if (realtimeConsumingTag != null) {
- if (!TagNameUtils.isServerTag(realtimeConsumingTag)) {
- throw new InvalidTableConfigException(
- "Invalid realtime consuming tag: " + realtimeConsumingTag + " for table " + tableNameWithType
- + ". Must have suffix _REALTIME or _OFFLINE");
- }
- if (getInstancesWithTag(realtimeConsumingTag).isEmpty()) {
- throw new InvalidTableConfigException(
- "No instances found with overridden realtime consuming tag: " + realtimeConsumingTag + " for table: "
- + tableNameWithType);
- }
+ Set<String> tagsToCheck = new TreeSet<>();
+ tagsToCheck.add(TagNameUtils.getBrokerTagForTenant(tenantConfig.getBroker()));
+ if (tableConfig.getTableType() == TableType.OFFLINE) {
+ tagsToCheck.add(new OfflineTagConfig(tableConfig).getOfflineServerTag());
+ } else {
+ RealtimeTagConfig realtimeTagConfig = new RealtimeTagConfig(tableConfig);
+ String consumingServerTag = realtimeTagConfig.getConsumingServerTag();
+ if (!TagNameUtils.isServerTag(consumingServerTag)) {
+ throw new InvalidTableConfigException(
+ "Invalid CONSUMING server tag: " + consumingServerTag + " for table: " + tableNameWithType);
}
-
- String realtimeCompletedTag = tagOverrideConfig.getRealtimeCompleted();
- if (realtimeCompletedTag != null) {
- if (!TagNameUtils.isServerTag(realtimeCompletedTag)) {
- throw new InvalidTableConfigException(
- "Invalid realtime completed tag: " + realtimeCompletedTag + " for table " + tableNameWithType
- + ". Must have suffix _REALTIME or _OFFLINE");
- }
- if (getInstancesWithTag(realtimeCompletedTag).isEmpty()) {
- throw new InvalidTableConfigException(
- "No instances found with overridden realtime completed tag: " + realtimeCompletedTag + " for table: "
- + tableNameWithType);
- }
+ tagsToCheck.add(consumingServerTag);
+ String completedServerTag = realtimeTagConfig.getCompletedServerTag();
+ if (!TagNameUtils.isServerTag(completedServerTag)) {
+ throw new InvalidTableConfigException(
+ "Invalid COMPLETED server tag: " + completedServerTag + " for table: " + tableNameWithType);
+ }
+ tagsToCheck.add(completedServerTag);
+ }
+ for (String tag : tagsToCheck) {
+ if (getInstancesWithTag(tag).isEmpty()) {
+ throw new InvalidTableConfigException(
+ "Failed to find instances with tag: " + tag + " for table: " + tableNameWithType);
}
}
}
@@ -1334,19 +1299,19 @@ public class PinotHelixResourceManager {
* Validate the table config and update it
* @throws IOException
*/
- public void updateTableConfig(TableConfig tableConfig, String tableNameWithType, TableType tableType)
+ public void updateTableConfig(TableConfig tableConfig)
throws IOException {
-
- validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- setExistingTableConfig(tableConfig, tableNameWithType, tableType);
+ validateTableTenantConfig(tableConfig);
+ setExistingTableConfig(tableConfig);
}
/**
* Sets the given table config into zookeeper
*/
- public void setExistingTableConfig(TableConfig tableConfig, String tableNameWithType, TableType tableType)
+ public void setExistingTableConfig(TableConfig tableConfig)
throws IOException {
-
+ String tableNameWithType = tableConfig.getTableName();
+ TableType tableType = tableConfig.getTableType();
if (tableType == TableType.REALTIME) {
IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
verifyIndexingConfig(tableNameWithType, indexingConfig);
@@ -1385,7 +1350,7 @@ public class PinotHelixResourceManager {
throw new RuntimeException("Table: " + tableName + " of type: " + type + " does not exist");
}
tableConfig.setCustomConfig(newConfigs);
- setExistingTableConfig(tableConfig, tableNameWithType, type);
+ setExistingTableConfig(tableConfig);
}
public void updateSegmentsValidationAndRetentionConfigFor(String tableName, TableType type,
@@ -1397,7 +1362,7 @@ public class PinotHelixResourceManager {
throw new RuntimeException("Table: " + tableName + " of type: " + type + " does not exist");
}
tableConfig.setValidationConfig(newConfigs);
- setExistingTableConfig(tableConfig, tableNameWithType, type);
+ setExistingTableConfig(tableConfig);
}
public void updateIndexingConfigFor(String tableName, TableType type, IndexingConfig newConfigs)
@@ -1408,7 +1373,7 @@ public class PinotHelixResourceManager {
throw new RuntimeException("Table: " + tableName + " of type: " + type + " does not exist");
}
tableConfig.setIndexingConfig(newConfigs);
- setExistingTableConfig(tableConfig, tableNameWithType, type);
+ setExistingTableConfig(tableConfig);
if (type == TableType.REALTIME) {
// Check if HLC table is allowed
@@ -1741,10 +1706,8 @@ public class PinotHelixResourceManager {
if (customConfig != null) {
Map<String, String> customConfigMap = customConfig.getCustomConfigs();
if (customConfigMap != null) {
- if (customConfigMap.containsKey(TableCustomConfig.MESSAGE_BASED_REFRESH_KEY) && !Boolean
- .valueOf(customConfigMap.get(TableCustomConfig.MESSAGE_BASED_REFRESH_KEY))) {
- return false;
- }
+ return !customConfigMap.containsKey(TableCustomConfig.MESSAGE_BASED_REFRESH_KEY) || Boolean
+ .valueOf(customConfigMap.get(TableCustomConfig.MESSAGE_BASED_REFRESH_KEY));
}
}
return true;
@@ -2067,12 +2030,16 @@ public class PinotHelixResourceManager {
: PinotResourceManagerResponse.failure("Timed out. External view not completely updated");
}
+ public boolean hasTable(String tableNameWithType) {
+ return getAllResources().contains(tableNameWithType);
+ }
+
public boolean hasOfflineTable(String tableName) {
- return getAllResources().contains(TableNameBuilder.OFFLINE.tableNameWithType(tableName));
+ return hasTable(TableNameBuilder.OFFLINE.tableNameWithType(tableName));
}
public boolean hasRealtimeTable(String tableName) {
- return getAllResources().contains(TableNameBuilder.REALTIME.tableNameWithType(tableName));
+ return hasTable(TableNameBuilder.REALTIME.tableNameWithType(tableName));
}
/**
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
index d130e31..ed76582 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManagerTest.java
@@ -43,7 +43,6 @@ import org.apache.pinot.common.config.TableNameBuilder;
import org.apache.pinot.common.config.TagNameUtils;
import org.apache.pinot.common.config.TagOverrideConfig;
import org.apache.pinot.common.config.Tenant;
-import org.apache.pinot.common.config.TenantConfig;
import org.apache.pinot.common.exception.InvalidConfigException;
import org.apache.pinot.common.metadata.ZKMetadataProvider;
import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata;
@@ -236,6 +235,9 @@ public class PinotHelixResourceManagerTest extends ControllerTest {
// Delete the table
_helixResourceManager.deleteOfflineTable(TABLE_NAME);
+
+ // Untag the brokers
+ untagBrokers();
}
@Test
@@ -304,164 +306,98 @@ public class PinotHelixResourceManagerTest extends ControllerTest {
Assert.assertEquals(serverTenantNames.iterator().next(), SERVER_TENANT_NAME);
_helixAdmin.removeInstanceTag(getHelixClusterName(), testServerInstance, "wrong_tag");
+
+ untagBrokers();
}
@Test
- public void testValidateTenantConfigs() {
- String tableNameWithType = "testTable_OFFLINE";
- TableType tableType = TableType.OFFLINE;
- int numReplica = 2;
- TableConfig tableConfig = null;
- String brokerTag = "aBrokerTag";
- String serverTag = "aServerTag";
-
- // null table config
- try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
- // expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
- }
+ public void testValidateTenantConfig() {
+ // Create broker tenant on 3 Brokers
+ Tenant brokerTenant =
+ new Tenant.TenantBuilder(BROKER_TENANT_NAME).setRole(TenantRole.BROKER).setTotalInstances(3).build();
+ _helixResourceManager.createBrokerTenant(brokerTenant);
- // null tenant config
- tableConfig = new TableConfig.Builder(tableType).setTableName(tableNameWithType).setNumReplicas(numReplica).build();
- try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
- // expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
- }
+ String rawTableName = "testTable";
+ TableConfig offlineTableConfig = new TableConfig.Builder(TableType.OFFLINE).setTableName(rawTableName).build();
- // null broker tenant
- TenantConfig tenantConfig = new TenantConfig();
- tenantConfig.setServer(serverTag);
- tableConfig.setTenantConfig(tenantConfig);
+ // Empty broker tag (DefaultTenant_BROKER)
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(offlineTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // null server tenant
- tenantConfig.setServer(null);
- tenantConfig.setBroker(brokerTag);
+ // Empty server tag (DefaultTenant_OFFLINE)
+ offlineTableConfig.getTenantConfig().setBroker(BROKER_TENANT_NAME);
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(offlineTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // empty broker instances list
- tenantConfig.setServer(serverTag);
- try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
- // expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
- }
+ // Valid tenant config without tagOverrideConfig
+ offlineTableConfig.getTenantConfig().setServer(SERVER_TENANT_NAME);
+ _helixResourceManager.validateTableTenantConfig(offlineTableConfig);
- // Create broker tenant on 3 Brokers
- Tenant brokerTenant = new Tenant.TenantBuilder(brokerTag).setRole(TenantRole.BROKER).setTotalInstances(3).build();
- _helixResourceManager.createBrokerTenant(brokerTenant);
+ TableConfig realtimeTableConfig =
+ new TableConfig.Builder(TableType.REALTIME).setTableName(rawTableName).setBrokerTenant(BROKER_TENANT_NAME)
+ .setServerTenant(SERVER_TENANT_NAME).build();
- // empty server instances list
+ // Empty server tag (serverTenant_REALTIME)
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(realtimeTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // valid tenant config, null tagOverrideConfig
- serverTag = SERVER_TENANT_NAME;
- tenantConfig.setServer(serverTag);
- try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- } catch (Exception e2) {
- Assert.fail("No exceptions expected");
- }
-
- // valid tagOverride config
+ // Incorrect CONSUMING server tag (serverTenant_BROKER)
TagOverrideConfig tagOverrideConfig = new TagOverrideConfig();
- tenantConfig.setTagOverrideConfig(tagOverrideConfig);
- try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- } catch (Exception e2) {
- Assert.fail("No exceptions expected");
- }
-
- // incorrect realtime consuming tag suffix
- tagOverrideConfig.setRealtimeConsuming("incorrectTag_XXX");
+ tagOverrideConfig.setRealtimeConsuming(TagNameUtils.getBrokerTagForTenant(SERVER_TENANT_NAME));
+ tagOverrideConfig.setRealtimeCompleted(TagNameUtils.getOfflineTagForTenant(SERVER_TENANT_NAME));
+ realtimeTableConfig.getTenantConfig().setTagOverrideConfig(tagOverrideConfig);
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(realtimeTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // incorrect realtime consuming tag suffix
- tagOverrideConfig.setRealtimeConsuming("correctTagEmptyList_OFFLINE");
+ // Empty CONSUMING server tag (serverTenant_REALTIME)
+ tagOverrideConfig.setRealtimeConsuming(TagNameUtils.getRealtimeTagForTenant(SERVER_TENANT_NAME));
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(realtimeTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // incorrect realtime completed tag suffix
- tagOverrideConfig.setRealtimeConsuming(serverTag + "_OFFLINE");
- tagOverrideConfig.setRealtimeCompleted("incorrectTag_XXX");
+ // Incorrect COMPLETED server tag (serverTenant_BROKER)
+ tagOverrideConfig.setRealtimeConsuming(TagNameUtils.getOfflineTagForTenant(SERVER_TENANT_NAME));
+ tagOverrideConfig.setRealtimeCompleted(TagNameUtils.getBrokerTagForTenant(SERVER_TENANT_NAME));
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(realtimeTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // empty list in realtime completed
- tagOverrideConfig.setRealtimeCompleted("correctTagEmptyList_OFFLINE");
+ // Empty COMPLETED server tag (serverTenant_REALTIME)
+ tagOverrideConfig.setRealtimeCompleted(TagNameUtils.getRealtimeTagForTenant(SERVER_TENANT_NAME));
try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
+ _helixResourceManager.validateTableTenantConfig(realtimeTableConfig);
Assert.fail("Expected InvalidTableConfigException");
- } catch (InvalidTableConfigException e1) {
+ } catch (InvalidTableConfigException e) {
// expected
- } catch (Exception e2) {
- Assert.fail("Expected InvalidTableConfigException");
}
- // all good
- tagOverrideConfig.setRealtimeCompleted(serverTag + "_OFFLINE");
- try {
- _helixResourceManager.validateTableTenantConfig(tableConfig, tableNameWithType, tableType);
- } catch (Exception e2) {
- Assert.fail("No exceptions expected");
- }
+ // Valid tenant config with tagOverrideConfig
+ tagOverrideConfig.setRealtimeCompleted(TagNameUtils.getOfflineTagForTenant(SERVER_TENANT_NAME));
+ _helixResourceManager.validateTableTenantConfig(realtimeTableConfig);
- for (String brokerInstance : _helixResourceManager.getAllInstancesForBrokerTenant(brokerTag)) {
- _helixAdmin
- .removeInstanceTag(getHelixClusterName(), brokerInstance, TagNameUtils.getBrokerTagForTenant(brokerTag));
- _helixAdmin.addInstanceTag(getHelixClusterName(), brokerInstance, CommonConstants.Helix.UNTAGGED_BROKER_INSTANCE);
- }
+ untagBrokers();
}
@Test
@@ -562,9 +498,7 @@ public class PinotHelixResourceManagerTest extends ControllerTest {
}
}
- @AfterMethod
- public void cleanUpBrokerTags() {
- // Untag all Brokers for other tests
+ private void untagBrokers() {
for (String brokerInstance : _helixResourceManager.getAllInstancesForBrokerTenant(BROKER_TENANT_NAME)) {
_helixAdmin.removeInstanceTag(getHelixClusterName(), brokerInstance,
TagNameUtils.getBrokerTagForTenant(BROKER_TENANT_NAME));
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/rebalance/ReplicaGroupRebalanceStrategyTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/rebalance/ReplicaGroupRebalanceStrategyTest.java
index 2350b87..1a5d017 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/rebalance/ReplicaGroupRebalanceStrategyTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/rebalance/ReplicaGroupRebalanceStrategyTest.java
@@ -92,14 +92,12 @@ public class ReplicaGroupRebalanceStrategyTest extends ControllerTest {
replicaGroupStrategyConfig.setNumInstancesPerPartition(numInstancesPerPartition);
replicaGroupStrategyConfig.setMirrorAssignmentAcrossReplicaGroups(true);
- String tableNameWithType = TableNameBuilder.OFFLINE.tableNameWithType(TABLE_NAME);
- TableConfig tableConfig = _helixResourceManager.getTableConfig(TABLE_NAME, CommonConstants.Helix.TableType.OFFLINE);
+ TableConfig tableConfig = _helixResourceManager.getOfflineTableConfig(TABLE_NAME);
tableConfig.getValidationConfig().setReplicaGroupStrategyConfig(replicaGroupStrategyConfig);
tableConfig.getValidationConfig().setSegmentAssignmentStrategy("ReplicaGroupSegmentAssignmentStrategy");
tableConfig.getValidationConfig().setReplication("2");
- _helixResourceManager
- .setExistingTableConfig(tableConfig, tableNameWithType, CommonConstants.Helix.TableType.OFFLINE);
+ _helixResourceManager.setExistingTableConfig(tableConfig);
// Test rebalancing after migration from non-replica to replica group table
_helixResourceManager.rebalanceTable(TABLE_NAME, CommonConstants.Helix.TableType.OFFLINE, rebalanceUserConfig);
@@ -302,13 +300,11 @@ public class ReplicaGroupRebalanceStrategyTest extends ControllerTest {
private void updateTableConfig(int targetNumInstancePerPartition, int targetNumReplicaGroup)
throws IOException {
- String tableNameWithType = TableNameBuilder.OFFLINE.tableNameWithType(TABLE_NAME);
- TableConfig tableConfig = _helixResourceManager.getTableConfig(TABLE_NAME, CommonConstants.Helix.TableType.OFFLINE);
+ TableConfig tableConfig = _helixResourceManager.getOfflineTableConfig(TABLE_NAME);
tableConfig.getValidationConfig().getReplicaGroupStrategyConfig()
.setNumInstancesPerPartition(targetNumInstancePerPartition);
tableConfig.getValidationConfig().setReplication(Integer.toString(targetNumReplicaGroup));
- _helixResourceManager
- .setExistingTableConfig(tableConfig, tableNameWithType, CommonConstants.Helix.TableType.OFFLINE);
+ _helixResourceManager.setExistingTableConfig(tableConfig);
}
@AfterClass
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/sharding/SegmentAssignmentStrategyTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/sharding/SegmentAssignmentStrategyTest.java
index 5376911..92ea2af 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/sharding/SegmentAssignmentStrategyTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/sharding/SegmentAssignmentStrategyTest.java
@@ -186,8 +186,7 @@ public class SegmentAssignmentStrategyTest extends ControllerTest {
replicaGroupTableConfig.getValidationConfig().setReplicaGroupStrategyConfig(replicaGroupStrategyConfig);
// Check that the replica group partition assignment is created
- _helixResourceManager
- .setExistingTableConfig(replicaGroupTableConfig, tableNameWithType, CommonConstants.Helix.TableType.OFFLINE);
+ _helixResourceManager.setExistingTableConfig(replicaGroupTableConfig);
partitionAssignment = _partitionAssignmentGenerator.getReplicaGroupPartitionAssignment(tableNameWithType);
Assert.assertTrue(partitionAssignment != null);
@@ -252,8 +251,7 @@ public class SegmentAssignmentStrategyTest extends ControllerTest {
// Check that the replica group partition assignment is created
try {
- _helixResourceManager
- .setExistingTableConfig(replicaGroupTableConfig, tableNameWithType, CommonConstants.Helix.TableType.REALTIME);
+ _helixResourceManager.setExistingTableConfig(replicaGroupTableConfig);
} catch (Exception e) {
// ignore
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org