Skip to content

Add OpChain converter SPI and adopt it in runtime paths.#17974

Merged
gortiz merged 3 commits intoapache:masterfrom
gortiz:opchain-spi-and-runtime-adoption
Mar 26, 2026
Merged

Add OpChain converter SPI and adopt it in runtime paths.#17974
gortiz merged 3 commits intoapache:masterfrom
gortiz:opchain-spi-and-runtime-adoption

Conversation

@gortiz
Copy link
Copy Markdown
Contributor

@gortiz gortiz commented Mar 25, 2026

Summary

  • Introduces an OpChain conversion SPI so Pinot can support alternative plan-to-OpChain implementations.
  • Adds a ServiceLoader-based dispatcher that selects the active converter (by priority or explicit override).
  • Migrates runtime conversion call sites to use the dispatcher, so the SPI is used in production execution paths.

Why

Pinot previously used a single hardcoded converter (PlanNodeToOpChain) in runtime execution paths. To enable extensibility (for example, alternative execution backends) without forking core runtime flow, we need a first-class conversion extension point and centralized dispatch. For example this can be a first step on the idea of fusing the timeseries engine with MSE.

Changes Included (with effect)

  • pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/OpChainConverter.java

    • Added new SPI interface for converting PlanNode to OpChain.
    • Added default implementation (DefaultOpChainConverter) that delegates to existing PlanNodeToOpChain.
    • Registered default implementation via @AutoService.
      • Effect: introduces an extension point while preserving current behavior as the default path.
  • pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/OpChainConverterDispatcher.java

    • Added dispatcher that discovers converters via ServiceLoader.
    • Added selection logic:
      • highest priority() wins by default
      • explicit converter override supported via setActiveConverterIdOverride(...)
      • deterministic tie-break by converter id
    • Added optimized fast path for default converter to avoid overhead on stock path.
      • Effect: centralizes converter selection and runtime dispatch, enabling pluggable conversion implementations.
  • pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/plan/OpChainConverterDispatcherTest.java

    • Added tests for converter registration/selection/override behavior.
      • Effect: validates SPI dispatch behavior and prevents regressions in converter resolution.
  • pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/QueryRunner.java

    • Replaced direct PlanNodeToOpChain.convert(...) call with dispatcher conversion.
      • Effect: non-leaf stage execution now honors active converter SPI.
  • pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/pipeline/PipelineBreakerExecutor.java

    • Replaced direct PlanNodeToOpChain.convert(...) with dispatcher conversion.
      • Effect: pipeline breaker execution now uses the active converter SPI.
  • pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/server/ServerPlanRequestUtils.java

    • Replaced direct PlanNodeToOpChain.convert(...) with dispatcher conversion.
      • Effect: server-side leaf stage compile path now uses the active converter SPI.
  • pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java

    • Replaced reduce-stage conversion call to use dispatcher conversion.
      • Effect: broker reducer conversion path now also uses active converter SPI, keeping conversion behavior consistent across runtime entry points.

Behavior / Compatibility

  • Default behavior remains unchanged because the default SPI implementation delegates to PlanNodeToOpChain.
  • No query semantics change is intended for existing deployments.
  • New functionality is additive: custom converters can be introduced as separate implementations discovered via ServiceLoader.

Test Plan

  • Unit tests:
    • ./mvnw -pl pinot-query-runtime -Dtest=OpChainConverterDispatcherTest test
  • Focused compile/tests for affected module:
    • ./mvnw -pl pinot-query-runtime -am -DskipTests compile
    • ./mvnw -pl pinot-query-runtime -am test -Dtest=QueryRunnerTest
  • Optional smoke:
    • Run a representative multistage query and verify execution/results are unchanged with default converter.

Introduce ServiceLoader-based conversion dispatch and route existing runtime conversion call sites through the SPI extension point.
@gortiz gortiz requested a review from yashmayya March 25, 2026 14:34
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 70.90909% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.26%. Comparing base (5b7d6fd) to head (776e341).
⚠️ Report is 28 commits behind head on master.

Files with missing lines Patch % Lines
...query/runtime/plan/OpChainConverterDispatcher.java 65.95% 9 Missing and 7 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17974      +/-   ##
============================================
- Coverage     63.30%   63.26%   -0.04%     
- Complexity     1525     1541      +16     
============================================
  Files          3193     3198       +5     
  Lines        193330   193990     +660     
  Branches      29732    29863     +131     
============================================
+ Hits         122381   122734     +353     
- Misses        61356    61614     +258     
- Partials       9593     9642      +49     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.24% <70.90%> (+0.04%) ⬆️
java-21 63.22% <70.90%> (-0.04%) ⬇️
temurin 63.26% <70.90%> (-0.04%) ⬇️
unittests 63.26% <70.90%> (-0.04%) ⬇️
unittests1 55.50% <70.90%> (-0.03%) ⬇️
unittests2 34.21% <0.00%> (-0.05%) ⬇️

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.

@yashmayya yashmayya added the multi-stage Related to the multi-stage query engine label Mar 26, 2026
@gortiz gortiz merged commit 022e6ad into apache:master Mar 26, 2026
29 of 32 checks passed
@gortiz gortiz deleted the opchain-spi-and-runtime-adoption branch March 26, 2026 15:00
xiangfu0 pushed a commit to pinot-contrib/pinot-docs that referenced this pull request Mar 26, 2026
@xiangfu0
Copy link
Copy Markdown
Contributor

Documentation for this feature has been added in pinot-contrib/pinot-docs#581.

The OpChainConverter SPI documentation includes:

  • Overview of the new SPI interface and its use cases
  • Implementation guide with code examples
  • Registration via @autoservice and Java ServiceLoader
  • Priority-based selection mechanism
  • Example custom converter implementation

See: pinot-contrib/pinot-docs#581

xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request Mar 27, 2026
…ity (apache/pinot#17974) (#581)

Co-authored-by: Pinot Docs Bot <docs-bot@pinot.apache.org>
@gortiz
Copy link
Copy Markdown
Contributor Author

gortiz commented Mar 27, 2026

I don't think this feature needs to be documented, but it won't hurt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-stage Related to the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants