You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2026/09/03 08:19:05 UTC
[PR] Remove the write-only op chain cache from OpChainSchedulerService [pinot]
gortiz opened a new pull request, #19443:
URL: https://github.com/apache/pinot/pull/19443
## What
Removes `_opChainCache` from `OpChainSchedulerService`. It has no reader.
## Why
The cache was added in #15609 so that `cancel()` could compute per-stage stats from the operator trees of op chains that had not finished, and return them to the broker:
```java
public Map<Integer, MultiStageQueryStats.StageStats.Closed> cancel(long requestId) {
...
e -> e.getValue().calculateStats().getCurrentStats().close(),
```
#18458 replaced that with per-op-chain stats pushed over the `SubmitWithStream` bidi RPC. `cancel()` became `void` and the `calculateStats()` read was deleted — its javadoc records the new contract: stats for cancelled op chains are not returned synchronously, stream mode pushes them per op chain, and "in legacy mode, cancel-path stats are not collected."
Nothing has read the cache since. On master the only remaining operations are one `put()` and four `invalidate()` calls.
## Why it is worth removing rather than leaving
`registerInternal()` puts every op chain's root operator and its `QueryExecutionContext` into the cache, and only the success path invalidates. An op chain that ends with an error block leaves its **entire operator tree and execution context reachable** until the entry is evicted by weight or by the 10 minute `expireAfterWrite` — for no reader. #18928 already had to add a fifth invalidation to the rejected-submit back-out path for the same reason.
It also imposes a locking contract that exists only to serve it: the `put()` has to stay inside the query read lock precisely so `cancel()`'s invalidation `forEach` can run outside the write lock and still observe every entry. That comment block goes away with the cache.
## Compatibility
`pinot.server.query.op.stats.cache.size` and `pinot.server.query.op.stats.cache.ms` are no longer read. Both constants are kept and marked `@Deprecated` so existing configurations keep starting rather than failing on an unknown key.
The two public constructors keep their signatures; only the private delegate loses the two parameters.
## Testing
`OpChainSchedulerServiceTest` passes unchanged (12/12) — no test asserted on the cache, which is itself consistent with it having no reader.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Remove the write-only op chain cache from OpChainSchedulerService [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz merged PR #19443:
URL: https://github.com/apache/pinot/pull/19443
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Remove the write-only op chain cache from OpChainSchedulerService [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19443:
URL: https://github.com/apache/pinot/pull/19443#issuecomment-5523425152
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19443?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 0.00%. Comparing base ([`46a07c7`](https://app.codecov.io/gh/apache/pinot/commit/46a07c77cfd199ab0013611d16435b95fc0e030b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`37974d9`](https://app.codecov.io/gh/apache/pinot/commit/37974d9414a6433259b82a1f657a923642d7d975?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 6 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (46a07c7) and HEAD (37974d9). Click for more details.
>
> <details><summary>HEAD has 18 uploads less than BASE</summary>
>
>| Flag | BASE (46a07c7) | HEAD (37974d9) |
>|------|------|------|
>|unittests1|1|0|
>|unittests|2|0|
>|java-25|6|2|
>|temurin|6|2|
>|lane-a|2|1|
>|integration|4|2|
>|integration1|2|0|
>|lane-b|2|1|
>|unittests2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19443 +/- ##
=============================================
- Coverage 67.63% 0.00% -67.64%
=============================================
Files 3487 3 -3484
Lines 224326 6 -224320
Branches 35408 0 -35408
=============================================
- Hits 151725 0 -151725
+ Misses 60573 6 -60567
+ Partials 12028 0 -12028
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-67.64%)` | :arrow_down: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [lane-b](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-67.64%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19443/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19443?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org