shounakmk219 opened a new pull request, #18580:
URL: https://github.com/apache/pinot/pull/18580
## Summary
Fixes regressions introduced by #16675 in `TableConfigsRestletResource`.
That PR added three new cluster-aware validation types — `TENANT`,
`MINION_INSTANCES`, `ACTIVE_TASKS` — to enhance the `/tableConfigs/validate`
endpoint (its stated intent). However, it wired them into the **shared**
private `validateConfig(TableConfigs, String, String)` helper, which is also
called from the create (`POST /tableConfigs`) and update (`PUT
/tableConfigs/{tableName}`) endpoints. This leaked the new checks onto paths
where they don't belong.
## Regressions observed
| # | Where | Description |
|---|---|---|
| **R1 (critical)** | `updateConfig` (PUT `/tableConfigs/{tableName}`) |
`tableTasksValidation` now runs on every update; it throws if **any** task
exists for the table, so any config update is blocked while tasks are in the
queue. The active-task check is only intended for create (catches stale state)
and delete (blocks deleting with running tasks). The sibling
`PinotTableRestletResource.updateTableConfig` at `PUT /tables/{tableName}`
correctly does not gate updates this way. |
| **R2** | `addConfig` (POST `/tableConfigs`) | `tableTasksValidation` is
called **twice** for offline / realtime configs — once via the new block in the
shared helper and again at the inline call sites in `addConfig`. |
| **R3** | `addConfig` | The pre-existing `@QueryParam("ignoreActiveTasks")`
flag is bypassed by the new call site; the helper gates only on the
`ACTIVE_TASKS` skip type, so `ignoreActiveTasks=true` no longer suppresses the
active-task check by itself — clients that relied on it broke. |
| **R4** | `addConfig` | `validateTableTenantConfig` +
`validateTableTaskMinionInstanceTagConfig` now run twice — once via the new
helper, and again inside `PinotHelixResourceManager.addTable(...)`. |
| **R5** | `updateConfig` | Same double-call as R4 — once via the new
helper, and again inside `PinotHelixResourceManager.updateTableConfig(...)`. |
R1–R3 are behavioral regressions affecting users; R4/R5 are duplicated
Helix/ZK calls per request.
## Fix
Keep the PR's intent (`/tableConfigs/validate` and `/tableConfigs/tune`
surface cluster-aware issues for fail-fast feedback), but stop leaking the new
checks onto create/update:
1. Remove the cluster-aware blocks from the shared
`validateConfig(TableConfigs, ...)` helper.
2. Extract a new private helper `validateClusterAwareConfig(TableConfig,
Set<ValidationType>)` that runs only the three cluster-aware checks, gated by
skip types.
3. Invoke the new helper only from `parseAndValidateTableConfigs` (the path
shared by `validateConfig` and `tuneConfig` endpoints). The `addConfig` and
`updateConfig` paths are not touched — they continue to rely on:
- The existing inline `tableTasksValidation` call in `addConfig`
(properly gated by `ignoreActiveTasks`).
- `PinotHelixResourceManager.addTable(...)` and `updateTableConfig(...)`
running tenant / minion checks internally (unchanged behavior, no double-call).
4. Revert the `@ApiParam` doc on `addConfig` and `updateConfig` back to
`(ALL|TASK|UPSERT)` since those endpoints don't consume the new skip types. The
validate/tune endpoints keep the expanded doc.
Net effect: `/tableConfigs/validate` and `/tableConfigs/tune` behave
identically to #16675. `addConfig` and `updateConfig` are restored to their
pre-#16675 behavior.
## Test plan
- [x] `./mvnw -pl pinot-controller -am compile` — succeeds
- [x] `./mvnw spotless:apply checkstyle:check license:check -pl
pinot-controller` — all clean
- [x] PR #16675's tests still pass:
`TableConfigsRestletResourceTest#testValidateConfigWithClusterValidationSkipTypes`
and `testValidateConfigClusterValidationsEnabled` (2 run, 0 failures)
- [ ] Manual: create a table with `taskConfig` + running task → `PUT
/tableConfigs/{tableName}` succeeds (pre-fix: blocked with "dangling task data")
- [ ] Manual: `POST /tableConfigs?ignoreActiveTasks=true` succeeds without
also needing `validationTypesToSkip=ACTIVE_TASKS`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
