Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ public ConfigSuccessResponse updateConfig(
@ManualAuthorization // performed after parsing TableConfigs
public String validateConfig(String tableConfigsStr,
@ApiParam(value = "comma separated list of validation type(s) to skip. supported types: "
+ "(ALL|TASK|UPSERT|TENANT|MINION_INSTANCES|ACTIVE_TASKS)")
+ "(ALL|TASK|UPSERT|TENANT|MINION_INSTANCES)")
@QueryParam("validationTypesToSkip") @Nullable String typesToSkip, @Context HttpHeaders httpHeaders,
@Context Request request) {
Pair<TableConfigs, Map<String, Object>> tableConfigsAndUnrecognizedProps =
Expand All @@ -479,7 +479,7 @@ public String validateConfig(String tableConfigsStr,
@ManualAuthorization // performed after parsing TableConfigs
public String tuneConfig(String tableConfigsStr,
@ApiParam(value = "comma separated list of validation type(s) to skip. supported types: "
+ "(ALL|TASK|UPSERT|TENANT|MINION_INSTANCES|ACTIVE_TASKS)")
+ "(ALL|TASK|UPSERT|TENANT|MINION_INSTANCES)")
@QueryParam("validationTypesToSkip") @Nullable String typesToSkip, @Context HttpHeaders httpHeaders,
@Context Request request) {
Pair<TableConfigs, Map<String, Object>> tableConfigsAndUnrecognizedProps =
Expand Down Expand Up @@ -515,8 +515,10 @@ private Pair<TableConfigs, Map<String, Object>> parseAndValidateTableConfigs(Str
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).
// feedback on tenant/minion issues without re-running them in the create/update paths (which already perform the
// equivalent checks inline or via PinotHelixResourceManager). Active-task validation is intentionally excluded
// here: it applies only on the create/update path (gated by the ignoreActiveTasks flag) so that validate/tune of an
// existing table with running tasks is not blocked.
Set<TableConfigUtils.ValidationType> skipTypes = TableConfigUtils.parseTypesToSkipString(typesToSkip);
try {
if (tableConfigs.getOffline() != null) {
Expand Down Expand Up @@ -553,9 +555,6 @@ private void validateClusterAwareConfig(TableConfig tableConfig, Set<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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -797,14 +797,16 @@ public void testValidateConfigWithClusterValidationSkipTypes()
adminClient.getTableClient().validateTableConfigs(tableConfigs.toPrettyJsonString(), "MINION_INSTANCES");
Assert.assertNotNull(responseWithMinionSkip);

// Test validation with ACTIVE_TASKS skip type - should pass even with potential task conflicts
// ACTIVE_TASKS is still accepted as a skip type for backward compatibility, but the validate/tune preflight
// endpoints no longer run active-task validation (it applies only on the create/update path), so passing it is a
// no-op here.
String responseWithTasksSkip =
adminClient.getTableClient().validateTableConfigs(tableConfigs.toPrettyJsonString(), "ACTIVE_TASKS");
Assert.assertNotNull(responseWithTasksSkip);

// Test validation with multiple skip types
String responseWithMultipleSkips = adminClient.getTableClient()
.validateTableConfigs(tableConfigs.toPrettyJsonString(), "TENANT,MINION_INSTANCES,ACTIVE_TASKS");
.validateTableConfigs(tableConfigs.toPrettyJsonString(), "TENANT,MINION_INSTANCES");
Assert.assertNotNull(responseWithMultipleSkips);

// Test validation with ALL skip type - should skip all validations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2067,6 +2067,10 @@ public static boolean isTableTypeInconsistentDuringConsumption(@Nullable TableCo
}

// enum of all the skip-able validation types.
// ACTIVE_TASKS is retained for backward compatibility: it is still accepted as a validationTypesToSkip value so
// existing clients do not break, but it is no longer consumed by any production path. Active-task validation runs
// only on the create/update path (see PinotTableRestletResource.tableTasksValidation, gated by ignoreActiveTasks),
// not on the validate/tune preflight endpoints, so passing ACTIVE_TASKS there is a no-op.
public enum ValidationType {
ALL, TASK, UPSERT, TENANT, MINION_INSTANCES, ACTIVE_TASKS
}
Expand Down
Loading