Skip to content

[bugfix] scope cluster-aware tableConfigs validations to validate/tune endpoints only #18580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shounakmk219 wants to merge 1 commit into
base: master
Choose a base branch
from
+36 −28
Open

[bugfix] scope cluster-aware tableConfigs validations to validate/tune endpoints only #18580

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 36 additions & 28 deletions .../src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,7 @@ public String getConfig(
@ManualAuthorization // performed after parsing table configs
public ConfigSuccessResponse addConfig(
String tableConfigsStr,
@ApiParam(value = "comma separated list of validation type(s) to skip. supported types: "
+ "(ALL|TASK|UPSERT|TENANT|MINION_INSTANCES|ACTIVE_TASKS)")
@ApiParam(value = "comma separated list of validation type(s) to skip. supported types: (ALL|TASK|UPSERT)")
@QueryParam("validationTypesToSkip") @Nullable String typesToSkip,
@DefaultValue("false") @QueryParam("ignoreActiveTasks") boolean ignoreActiveTasks,
@Context HttpHeaders httpHeaders, @Context Request request)
Expand Down Expand Up @@ -366,8 +365,7 @@ public SuccessResponse deleteConfig(
public ConfigSuccessResponse updateConfig(
@ApiParam(value = "TableConfigs name i.e. raw table name", required = true) @PathParam("tableName")
String tableName,
@ApiParam(value = "comma separated list of validation type(s) to skip. supported types: "
+ "(ALL|TASK|UPSERT|TENANT|MINION_INSTANCES|ACTIVE_TASKS)")
@ApiParam(value = "comma separated list of validation type(s) to skip. supported types: (ALL|TASK|UPSERT)")
@QueryParam("validationTypesToSkip") @Nullable String typesToSkip,
@ApiParam(value = "Reload the table if the new schema is backward compatible") @DefaultValue("false")
@QueryParam("reload") boolean reload,
Expand Down Expand Up @@ -516,6 +514,25 @@ private Pair<TableConfigs, Map<String, Object>> parseAndValidateTableConfigs(Str
String rawTableName = DatabaseUtils.translateTableName(tableConfigs.getTableName(), databaseName);
tableConfigs.setTableName(rawTableName);

// Cluster-aware validations are exclusive to the validate/tune pre-flight endpoints so that users get fail-fast
// feedback on tenant/minion/active-task issues without re-running them in the create/update paths (which already
// perform the equivalent checks inline or via PinotHelixResourceManager).
Set<TableConfigUtils.ValidationType> skipTypes = TableConfigUtils.parseTypesToSkipString(typesToSkip);
try {
if (tableConfigs.getOffline() != null) {
validateClusterAwareConfig(tableConfigs.getOffline(), skipTypes);
}
if (tableConfigs.getRealtime() != null) {
validateClusterAwareConfig(tableConfigs.getRealtime(), skipTypes);
}
} catch (ControllerApplicationException e) {
// Already logged by the inner constructor; let it propagate as-is.
throw e;
} catch (Exception e) {
throw new ControllerApplicationException(LOGGER,
String.format("Invalid TableConfigs: %s. %s", rawTableName, e.getMessage()), Response.Status.BAD_REQUEST, e);
}

// validate permission
String endpointUrl = request.getRequestURL().toString();
AccessControl accessControl = _accessControlFactory.create();
Expand All @@ -526,6 +543,21 @@ private Pair<TableConfigs, Map<String, Object>> parseAndValidateTableConfigs(Str
return tableConfigsAndUnrecognizedProps;
}

private void validateClusterAwareConfig(TableConfig tableConfig, Set<TableConfigUtils.ValidationType> skipTypes) {
if (skipTypes.contains(TableConfigUtils.ValidationType.ALL)) {
return;
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.TENANT)) {
_pinotHelixResourceManager.validateTableTenantConfig(tableConfig);
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.MINION_INSTANCES)) {
_pinotHelixResourceManager.validateTableTaskMinionInstanceTagConfig(tableConfig);
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.ACTIVE_TASKS)) {
PinotTableRestletResource.tableTasksValidation(tableConfig, _pinotHelixTaskResourceManager);
}
}

private void applyTuning(TableConfig tableConfig, Schema schema) {
TableConfigTunerUtils.applyTunerConfigs(_pinotHelixResourceManager, tableConfig, schema, Collections.emptyMap());
TableConfigUtils.ensureMinReplicas(tableConfig, _controllerConf.getDefaultTableMinReplicas());
Expand Down Expand Up @@ -553,26 +585,13 @@ private void validateConfig(TableConfigs tableConfigs, String database, @Nullabl
Preconditions.checkState(rawTableName.equals(schemaName),
"'tableName': %s must be equal to 'schemaName' from 'schema': %s", rawTableName, schema.getSchemaName());
SchemaUtils.validate(schema);
// Parse validation types to skip using TableConfigUtils method
Set<TableConfigUtils.ValidationType> skipTypes = TableConfigUtils.parseTypesToSkipString(typesToSkip);
if (offlineTableConfig != null) {
String offlineRawTableName = DatabaseUtils.translateTableName(
TableNameBuilder.extractRawTableName(offlineTableConfig.getTableName()), database);
Preconditions.checkState(offlineRawTableName.equals(rawTableName),
"Name in 'offline' table config: %s must be equal to 'tableName': %s", offlineRawTableName, rawTableName);
TableConfigUtils.validateTableName(offlineTableConfig);
TableConfigUtils.validate(offlineTableConfig, schema, typesToSkip);
if (!skipTypes.contains(TableConfigUtils.ValidationType.ALL)) {
if (!skipTypes.contains(TableConfigUtils.ValidationType.TENANT)) {
_pinotHelixResourceManager.validateTableTenantConfig(offlineTableConfig);
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.MINION_INSTANCES)) {
_pinotHelixResourceManager.validateTableTaskMinionInstanceTagConfig(offlineTableConfig);
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.ACTIVE_TASKS)) {
PinotTableRestletResource.tableTasksValidation(offlineTableConfig, _pinotHelixTaskResourceManager);
}
}
TaskConfigUtils.validateTaskConfigs(tableConfigs.getOffline(), schema, _pinotTaskManager, typesToSkip);
TableConfigValidatorRegistry.validate(offlineTableConfig, schema);
}
Expand All @@ -583,17 +602,6 @@ private void validateConfig(TableConfigs tableConfigs, String database, @Nullabl
"Name in 'realtime' table config: %s must be equal to 'tableName': %s", realtimeRawTableName, rawTableName);
TableConfigUtils.validateTableName(realtimeTableConfig);
TableConfigUtils.validate(realtimeTableConfig, schema, typesToSkip);
if (!skipTypes.contains(TableConfigUtils.ValidationType.ALL)) {
if (!skipTypes.contains(TableConfigUtils.ValidationType.TENANT)) {
_pinotHelixResourceManager.validateTableTenantConfig(realtimeTableConfig);
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.MINION_INSTANCES)) {
_pinotHelixResourceManager.validateTableTaskMinionInstanceTagConfig(realtimeTableConfig);
}
if (!skipTypes.contains(TableConfigUtils.ValidationType.ACTIVE_TASKS)) {
PinotTableRestletResource.tableTasksValidation(realtimeTableConfig, _pinotHelixTaskResourceManager);
}
}
TaskConfigUtils.validateTaskConfigs(tableConfigs.getRealtime(), schema, _pinotTaskManager, typesToSkip);
TableConfigValidatorRegistry.validate(realtimeTableConfig, schema);
}
Expand Down
Loading