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: [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]
