[pinot-server/ proactive-query-killing] (1/2) add initial SPI implementation for supporting query killing based on Scan Cost by anuragrai16 · Pull Request #18102 · apache/pinot

anuragrai16 · 2026-04-06T08:58:22Z

Implementation for #18043

This PR introduces all core classes for a proactive, scan-cost-based query killing framework. No existing query execution path is modified in this PR. A follow-up PR will wire these into BaseOperator.checkTermination() and the operator instrumentation points.

New classes:

QueryScanCostContext (pinot-spi) — LongAdder-based, thread-safe accumulator for numEntriesScannedInFilter, numDocsScanned, and
numEntriesScannedPostFilter. One instance per query, shared across segment worker threads.
QueryKillingStrategy (pinot-core) — Interface for pluggable kill decisions. Includes shouldTerminate(), buildKillReport(),
getErrorCode(), and forQuery() for table-level threshold overrides.
ScanEntriesThresholdStrategy (pinot-core) — Default strategy. Kills queries exceeding maxEntriesScannedInFilter or maxDocsScanned.
Threshold of Long.MAX_VALUE disables that metric. Includes a nested Factory that validates config at init and logs a warning if no
thresholds are set.
CompositeQueryKillingStrategy (pinot-core) — Combines strategies with AND/OR semantics.
QueryKillingStrategyFactory (pinot-core) — Factory interface for config-driven strategy loading. Custom strategies are plugged in by
setting accounting.scan.based.killing.strategy.factory.class.name to a factory class
QueryKillReport (pinot-core) — Immutable snapshot of a kill event. Captures all metrics at creation time
Produces customer-facing error messages with actionable advice and structured internal log messages.
QueryKillingManager (pinot-core) — Singleton manager. Builds strategy once at init via factory, delegates
shouldTerminate()/buildKillReport() to the strategy.

Table-Level overrides supported
Table-level overrides via QueryConfig. Table config takes precedence over cluster config; null means use cluster default.

Error code: QUERY_SCAN_LIMIT_EXCEEDED (246, HTTP 400) - distinguishes scan kills (client problem) from OOM kills (server problem, 503).

Minimum config to enable

pinot.query.scheduler.accounting.scan.based.killing.enabled=true
pinot.query.scheduler.accounting.scan.based.killing.max.entries.scanned.in.filter=500000000

…scan cost

codecov-commenter · 2026-04-06T10:03:03Z

Codecov Report

❌ Patch coverage is 88.88889% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.62%. Comparing base (78e38eb) to head (5757849).
⚠️ Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
.../pinot/core/query/killing/QueryKillingManager.java	74.13%	12 Missing and 3 partials ⚠️
...killing/strategy/ScanEntriesThresholdStrategy.java	90.00%	0 Missing and 4 partials ⚠️
...ache/pinot/core/accounting/QueryMonitorConfig.java	93.87%	1 Missing and 2 partials ⚠️
...e/query/killing/CompositeQueryKillingStrategy.java	92.00%	1 Missing and 1 partial ⚠️
...va/org/apache/pinot/spi/utils/CommonConstants.java	86.66%	1 Missing and 1 partial ⚠️
...pinot/core/query/killing/QueryKillingStrategy.java	66.66%	1 Missing ⚠️
...org/apache/pinot/spi/config/table/QueryConfig.java	91.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18102      +/-   ##
============================================
+ Coverage     63.58%   63.62%   +0.04%     
+ Complexity     1659     1653       -6     
============================================
  Files          3245     3252       +7     
  Lines        197441   197760     +319     
  Branches      30564    30614      +50     
============================================
+ Hits         125536   125826     +290     
- Misses        61856    61874      +18     
- Partials      10049    10060      +11

Flag	Coverage Δ
custom-integration1	`?`
integration	`?`
integration1	`?`
integration2	`?`
java-11	`55.60% <88.88%> (+0.09%)`	⬆️
java-21	`63.59% <88.88%> (+0.03%)`	⬆️
temurin	`63.62% <88.88%> (+0.04%)`	⬆️
unittests	`63.62% <88.88%> (+0.04%)`	⬆️
unittests1	`55.62% <88.88%> (+0.08%)`	⬆️
unittests2	`35.00% <5.15%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tarun11Mavani · 2026-04-19T17:46:06Z

  // CPU time based killing threshold
  private final long _cpuTimeBasedKillingThresholdNs;

  private final boolean _queryKilledMetricEnabled;


Side q: Are we honoring this flag during emitting the metics in future PRs? worth a look.

No, this flag doesn't make sense tbh. I always publish metrics when the feature is enabled, and the metric differentiates between logOnly (QUERY_KILLED_DRY_RUN) and enforce (QUERY_KILLED). Not sure why we would want to disable the metrics when the feature is enabled and seems like a redundant extra config.

tarun11Mavani · 2026-04-19T17:47:41Z

 public class QueryMonitorConfig {
+  private static final Logger LOGGER = LoggerFactory.getLogger(QueryMonitorConfig.class);
+
+  private static final Set<String> VALID_SCAN_KILLING_MODES = Set.of(


Can we replace this and _scanBasedKillingMode with public enum ScanKillingMode { DISABLED, LOG_ONLY, ENFORCE }

Makes sense, updated

tarun11Mavani · 2026-04-19T17:48:51Z

+   * (disabled, logOnly, enforce), logs an error and falls back to "disabled" so the server
+   * continues to start normally.
+   */
+  private static String validateScanKillingMode(String mode) {


This can be removed if we use enum.

shauryachats · 2026-04-21T02:51:54Z

+
+    if (changedConfigs.contains(
+        CommonConstants.Accounting.CONFIG_OF_SCAN_BASED_KILLING_MAX_ENTRIES_SCANNED_POST_FILTER)) {
+      _scanBasedKillingMaxEntriesScannedPostFilter = Long.parseLong(


Guard against invalid string values (empty string for example).

thanks, handled

shauryachats · 2026-04-21T02:52:45Z

+            factory.getName());
+      }
+    } catch (Exception e) {
+      LOGGER.error("Failed to initialize scan-based killing strategy. "


Can we also add metrics here for production monitoring?

Hmm, this metric will only emit one time and not really a timeseries, dont see a reason for a metric here. The log should be enough to let me know that the strategy failed to initialize, and we will continue with query killing being a no-op

shauryachats · 2026-04-21T03:01:35Z

+public class QueryKillingManager {
+  private static final Logger LOGGER = LoggerFactory.getLogger(QueryKillingManager.class);
+
+  private static volatile QueryKillingManager _instance;


Any particular reason we are keeping a static volatile variable and not passing it to relevant classes through DI?

The consumer for this (to be added in next diff) is BaseOperator.checkTermination(), a method called on every nextBlock() invocation across many operator subclasses. Threading the manager through constructors would mean modifying the constructor of every operator, much larger change to review.
2. The manager is also server-scoped, we want to create one instance per JVM, initialized once at startup (BaseServerStarter), and then use it everywhere.

…rror metric Replace the three string constants (disabled/logOnly/enforce) with a proper ScanKillingMode enum for compile-time safety and cleaner comparisons. The enum supports case-insensitive parsing from config values for backward compatibility. Also add QUERIES_KILLED_SCAN_ERROR metric emitted when scan-based killing evaluation throws an unexpected exception. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add parseLongOrDefault() helper that safely handles null, empty string, whitespace, and non-numeric values when parsing scan killing thresholds from ZK cluster config updates. Falls back to the default (Long.MAX_VALUE) with an error log instead of throwing NumberFormatException. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

add initial SPI implementation for supporting query killing based on …

4fb9c6f

…scan cost

anuragrai16 changed the title ~~[pinot-server/ proactive-query-killing] add initial SPI implementation for supporting query killing based on …~~ [pinot-server/ proactive-query-killing] (1/2) add initial SPI implementation for supporting query killing based on … Apr 6, 2026

anuragrai16 added 3 commits April 6, 2026 14:39

checkstyle fix

68d8a2f

fix dcos

49e3ce0

fix docs 2

36a6c2b

anuragrai16 changed the title ~~[pinot-server/ proactive-query-killing] (1/2) add initial SPI implementation for supporting query killing based on …~~ [pinot-server/ proactive-query-killing] (1/2) add initial SPI implementation for supporting query killing based on Scan Cost Apr 6, 2026

convert to single config that takes logOnly v/s enforced mode

33667ec

tarun11Mavani reviewed Apr 20, 2026

View reviewed changes

shauryachats reviewed Apr 21, 2026

View reviewed changes

anuragrai16 and others added 8 commits April 22, 2026 22:55

Merge branch 'master' into proactiveQueryKillingSPI

7fff682

Fix error in merge

c2f67e1

Revert wrong merge

2b5a5d4

Update QueryMonitorConfig.java

2dfe958

spotless fix

f6bb778

make QueryConfig constructor backward compatible

5757849

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pinot-server/ proactive-query-killing] (1/2) add initial SPI implementation for supporting query killing based on Scan Cost#18102

[pinot-server/ proactive-query-killing] (1/2) add initial SPI implementation for supporting query killing based on Scan Cost#18102
anuragrai16 wants to merge 13 commits intoapache:masterfrom
anuragrai16:proactiveQueryKillingSPI

anuragrai16 commented Apr 6, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Apr 6, 2026 •

edited

Loading

Uh oh!

tarun11Mavani Apr 19, 2026

Uh oh!

anuragrai16 Apr 22, 2026

Uh oh!

tarun11Mavani Apr 19, 2026

Uh oh!

anuragrai16 Apr 22, 2026

Uh oh!

tarun11Mavani Apr 19, 2026

Uh oh!

Uh oh!

shauryachats Apr 21, 2026

Uh oh!

anuragrai16 Apr 22, 2026

Uh oh!

shauryachats Apr 21, 2026

Uh oh!

anuragrai16 Apr 22, 2026

Uh oh!

shauryachats Apr 21, 2026

Uh oh!

anuragrai16 Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

anuragrai16 commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anuragrai16 commented Apr 6, 2026 •

edited

Loading

codecov-commenter commented Apr 6, 2026 •

edited

Loading