deepthi912 opened a new pull request, #18762: URL: https://github.com/apache/pinot/pull/18762
## Summary Adds two nullable fields to `tableConfig.queryConfig` so operators can opt individual tables in or out of the pre-OOM-kill query pause behavior independently of the server-instance defaults: - `oomPreQueryKillPauseDurationMs` (`Long`) - `oomPanicAllowPreQueryKillPause` (`Boolean`) Both default to `null`. Because `BaseJsonConfig` is annotated `@JsonInclude(NON_NULL)`, existing table configs serialize byte-for-byte identically — no field rename, no enum extension, no protocol change. ## Motivation The existing OOM pre-kill pause configs (`pinot.server.query.accounting.oom.pre.query.kill.pause.duration.ms` and `oom.panic.allow.pre.query.kill.pause`) are read once at server startup into `QueryMonitorConfig` and apply JVM-wide. That's fine for dedicated servers, but on mixed-tenant servers the pause behavior leaks to every co-hosted table. For workloads that read large objects from object storage on the query path (e.g. external / Iceberg-backed tables), the pause gives in-flight work a chance to release memory before the kill — but co-hosted tables with smaller working sets don't necessarily want that pause. Per-table opt-in lets the table author decide. The server-instance defaults remain the fallback for any table that doesn't override. ## What this PR does SPI only — adds the two fields, getters, a constructor overload that preserves the existing 9-arg signature, and a precondition rejecting negative pause durations (`0` is allowed as an explicit opt-out signal). ## What this PR does NOT do (follow-up) The consumer side in `QueryResourceAggregator` is **not** in this PR. A follow-up will: 1. Inject `TableCache` into `ResourceUsageAccountantFactory`. 2. In `preAggregate`/`postAggregate` panic and critical paths, walk `threadTrackers`, resolve each query's `TableConfig.queryConfig`, and decide pause-vs-kill per query — falling back to the JVM-wide `QueryMonitorConfig` when the table fields are `null`. Splitting the PRs keeps the SPI change reviewable on its own and lets the per-query accountant logic be discussed independently. ## Tests New `QueryConfigOomPauseTest` covers: - Default-null state. - Explicit positive and zero values (zero as the opt-out signal). - JSON round-trip with the fields set. - JSON deserialization with the fields absent or set in isolation. - Negative-value rejection via `Preconditions.checkArgument`. All 15 tests under `QueryConfig*Test` pass. ## Backward compatibility - Existing JSON: any table config that doesn't include the new keys deserializes to `null`/`null` and re-serializes without them. - Existing Java callers: the 9-arg constructor is preserved as an overload that defers to the new 11-arg `@JsonCreator` constructor with `null, null`. `OfflineClusterIntegrationTest`, `BaseLogicalTableIntegrationTest`, `QueryKillingManagerTest`, etc. compile unchanged. - Rolling upgrade: nullable fields, no protocol bump, no migration required. ## Test plan - [x] `./mvnw -pl pinot-spi -Dtest='QueryConfig*Test' test` — 15/15 passing - [x] `./mvnw -pl pinot-spi checkstyle:check` — clean - [x] `./mvnw -pl pinot-spi spotless:apply license:format` — applied - [ ] CI green -- 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]
