mcvsubbu commented on a change in pull request #4103: Add validations to table
config before updating a table config
URL: https://github.com/apache/incubator-pinot/pull/4103#discussion_r274496234
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -1160,7 +1113,60 @@ public void addTable(@Nonnull TableConfig tableConfig)
throw new InvalidTableConfigException("UnSupported table type: " +
tableType);
}
- handleBrokerResource(tableNameWithType, brokersForTenant);
+ String brokerTenantName =
TagNameUtils.getBrokerTagForTenant(tableConfig.getTenantConfig().getBroker());
+ handleBrokerResource(tableNameWithType,
HelixHelper.getInstancesWithTag(_helixZkManager, brokerTenantName));
+ }
+
+ /**
+ * Validates the tenant config for the table
+ */
+ private void validateTableTenantConfig(TableConfig tableConfig, String
tableNameWithType, TableType tableType) {
+ TenantConfig tenantConfig = tableConfig.getTenantConfig();
+ if (tenantConfig == null || tenantConfig.getBroker() == null ||
tenantConfig.getServer() == null) {
+ throw new PinotHelixResourceManager.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 =
HelixHelper.getInstancesWithTag(_helixZkManager, brokerTenantName);
+ if (brokersForTenant.isEmpty()) {
+ throw new PinotHelixResourceManager.InvalidTableConfigException(
+ "Broker tenant: " + brokerTenantName + " does not exist for table: "
+ tableNameWithType);
+ }
+ String serverTenantName =
+ TagNameUtils.getTagFromTenantAndServerType(tenantConfig.getServer(),
tableType.getServerType());
+ if (HelixHelper.getInstancesWithTag(_helixZkManager,
serverTenantName).isEmpty()) {
+ throw new PinotHelixResourceManager.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.hasValidServerTagSuffix(realtimeConsumingTag)) {
+ throw new PinotHelixResourceManager.InvalidTableConfigException(
+ "Invalid realtime consuming tag: " + realtimeConsumingTag + ".
Must have suffix _REALTIME or _OFFLINE");
+ }
+ if (HelixHelper.getInstancesWithTag(_helixZkManager,
realtimeConsumingTag).isEmpty()) {
+ throw new PinotHelixResourceManager.InvalidTableConfigException(
+ "No instances found with overridden realtime consuming tag: " +
realtimeConsumingTag + " for table: "
+ + tableNameWithType);
+ }
+ }
+
+ String realtimeCompletedTag = tagOverrideConfig.getRealtimeCompleted();
+ if (realtimeCompletedTag != null) {
+ if (!TagNameUtils.hasValidServerTagSuffix(realtimeCompletedTag)) {
+ throw new PinotHelixResourceManager.InvalidTableConfigException(
+ "Invalid realtime completed tag: " + realtimeCompletedTag + ".
Must have suffix _REALTIME or _OFFLINE");
Review comment:
include table name in the exception
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
