gortiz opened a new pull request, #18736:
URL: https://github.com/apache/pinot/pull/18736
#18458 introduced the `OperatorTypeDescriptor` SPI so plugin jars can
register custom MSE operator types (ids ≥ 256) and report their stats over the
`SubmitWithStream` channel. While exercising that SPI from an out-of-tree
plugin extension we found the plugin path does not work end to end — every
query whose stages report plugin-typed stats returned `stageStats: {"error":
"... java.lang.ArithmeticException: / by zero"}`, and on classloader-isolated
deployments the descriptors were never registered at all.
## Root causes
1. The broker renders `stageStats` by pairing the flat per-stage stats list
positionally against the stage's `PlanNode` tree (`InStageStatsTreeBuilder`),
assuming built-in types at every position. `visitMailboxSend` casts the
send-position StatMap to the built-in key class; for a plugin type the
`EnumMap` lookup returns 0, so `Context(parallelism=0)` makes `cpuTimeMs /
parallelism` throw. Independently, the type-mismatch branch silently drops
every plugin-typed node, so even without the exception the plugin stats would
never appear.
2. `OperatorTypeRegistry` discovers plugin descriptors with the plain
context-classpath `ServiceLoader`, unlike `PinotRuleSet`, which also walks the
`PluginManager` plugin classloaders.
## Fixes
- Stream-mode stages are now rendered **directly from the decoded
`StageStatsTreeNode`** — the explicit tree shape is exactly what the new wire
format carries, so no shape re-derivation is needed and plugin types render
faithfully. Cross-stage nesting (sender stage under its mailbox-receive node)
is resolved through plan-node ids; the broker reproduces the same pre-order
numbering the servers use. `PIPELINE_BREAKER` nodes are collapsed so the JSON
shape stays bit-compatible with the legacy renderer (external tooling parses
this format). Nodes additionally expose `planNodeIds`.
- `OperatorTypeRegistry` now walks
`PluginManager.get().getPluginClassLoaders()` (deduplicated by provider class
name), matching the `PinotRuleSet` pattern.
- New `OpChainExecutionContext#getPlanNodeIdMap()` accessor so plugin
operator trees can register synthetic stats operators for id resolution via the
existing `recordPlanNodesForOperator`.
- Defensive fix on the legacy path: `visitMailboxSend` only reads
`PARALLELISM` from genuine built-in `MAILBOX_SEND` stats and clamps the divisor
to ≥ 1.
## Test plan
- [x] New `MultiStageStatsTreeBuilderTest` — explicit-tree rendering with
plugin-only operator types carrying a plugin-defined `StatMap.Key` enum
(cross-stage nesting by plan-node id, `PIPELINE_BREAKER` collapse parity,
legacy-path `/0` regression)
- [x] `StreamStatsReportingIntegrationTest` — 7/7 (including the
pipeline-breaker shape tests, which pin JSON parity of the new renderer)
- [x] `MultiStageStatsTreeEncoderTest` / `MultiStageStatsTreeDecoderTest` /
`StreamingQuerySessionTest` / `OpChainSchedulerServiceTest` /
`OperatorTypeRegistryTest` — all green
- [x] Verified end to end against the out-of-tree plugin extension:
plugin-typed stages render in `stageStats` with correct cross-stage nesting and
no errors
🤖 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]
