This is an automated email from the ASF dual-hosted git repository.
xiangfu0 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 dbeb3940cf7 [bugfix] scope cluster-aware tableConfigs validations to
validate/tune endpoints only (#18580)
dbeb3940cf7 is described below
commit dbeb3940cf71f50d1b6e7930a88dfe840505077f
Author: Shounak kulkarni <[email protected]>
AuthorDate: Thu May 28 21:29:34 2026 +0530
[bugfix] scope cluster-aware tableConfigs validations to validate/tune
endpoints only (#18580)
PR #16675 placed the new TENANT/MINION_INSTANCES/ACTIVE_TASKS checks
inside the shared private validateConfig helper, so they leaked onto
the create and update paths. Move them into a new helper invoked only
from parseAndValidateTableConfigs (the validate/tune flow). This
restores update flow behavior (active-task check no longer blocks
updates), drops the duplicate tableTasksValidation/tenant/minion calls
on create, and keeps the PR's intent of fail-fast pre-flight feedback
on /tableConfigs/validate and /tableConfigs/tune intact.
Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
---
.../api/resources/TableConfigsRestletResource.java | 64 ++++++++++++----------
1 file changed, 36 insertions(+), 28 deletions(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
index e37ee4d086f..d6fc46c3adb 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
@@ -198,8 +198,7 @@ public class TableConfigsRestletResource {
@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)
@@ -366,8 +365,7 @@ public class TableConfigsRestletResource {
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,
@@ -516,6 +514,25 @@ public class TableConfigsRestletResource {
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();
@@ -526,6 +543,21 @@ public class TableConfigsRestletResource {
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());
@@ -553,8 +585,6 @@ public class TableConfigsRestletResource {
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);
@@ -562,17 +592,6 @@ public class TableConfigsRestletResource {
"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);
}
@@ -583,17 +602,6 @@ public class TableConfigsRestletResource {
"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);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
