anshul98ks123 opened a new pull request, #19660: URL: https://github.com/apache/pinot/pull/19660
## Summary Ad-hoc `PinotTaskManager#createTask` is the one task-generation entry point that still takes the controller-wide monitor unconditionally. With `controller.task.concurrentSchedulingEnabled`, cron generation for different tables already runs in parallel, but an ad-hoc request for one table still queues behind any other table's ad-hoc generation. That matters because some generators do their expensive work inside it. A generator that lists an input prefix can hold the monitor for minutes on a large prefix, and every other ad-hoc request on the controller — for unrelated tables — waits the whole time, often past the client's timeout, without doing any work of its own. ## Change `createTask` now dispatches the way `scheduleTasks` already does: - **Concurrent path** — when every table the request targets resolves to concurrent scheduling (the `TableTaskConfig#concurrentSchedulingEnabled` override, else the cluster default) **and** distributed locking is enabled, the call holds no controller-wide monitor. - **Legacy path** — otherwise, it runs under `synchronized(this)` exactly as before. Both paths share `doCreateTask`, which is the previous method body unchanged. The `TableTaskConfig` docs now note that the flag covers ad-hoc creation as well as cron scheduling. ### Why distributed locking is required here `shouldUseConcurrentPath` only recommends distributed locking. For ad-hoc creation it is required: once the monitor is gone, the per-table ZK lock is the only thing keeping two ad-hoc requests for the **same** table from generating tasks side by side. Without it the monitor stays, so same-table exclusion is never weaker than today. ### What runs without the monitor Nothing new. Every step `doCreateTask` takes — `prepTaskQueue`, `isTaskSchedulable`, `acquireTaskLock`, `generateTasks`, `validatePinotTaskConfigs`, `addDefaultsToTaskConfig`, `submitTasks` — is already run without the monitor by the concurrent cron path in `doScheduleTasks`. This change adds a caller, not a new unlocked step. ## Backward compatibility No behaviour change by default: both `controller.task.concurrentSchedulingEnabled` and `controller.task.enableDistributedLocking` default to `false`, so every request takes the legacy path. An unresolvable table name also takes the legacy path and fails there exactly as it did before. ## Testing Nine new cases in `PinotTaskManagerConcurrentSchedulingTest`: - **Resolver** — table opt-in, cluster default inherited, table opt-out over cluster default, no distributed locking, hybrid table with one type opted out, unknown table, missing table config. - **Behaviour** — table A's generation parks inside `createTask`; a request for table B then either proceeds (both opted in) or is `BLOCKED` on the monitor (not opted in). The opted-in behavioural test fails against the previous code — *"table B waited on table A although both opted into concurrent scheduling"* — and passes with this change. `spotless`, `checkstyle` and `license` are clean on `pinot-spi` and `pinot-controller`. 🤖 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]
