vrajat opened a new pull request, #16299:
URL: https://github.com/apache/pinot/pull/16299
In critical mode, the accountant sends a cancel to the most expensive query.
This query may not terminate immediately. So the accountant will attempt
multiple times to cancel the query. A sinister side effect is that it also
increments `_numQueriesKilledConsecutively` and calls GC when the value is
above a config. Note that the GC call is useless as the query hasnt cancelled
yet and released the memory allocations.
The accountant now remembers the queries that have been cancelled in
`_cancelSentQueries`. **It finds the most expensive query which hasn't been
cancelled.**
```
maxUsageTuple = _aggregatedUsagePerActiveQuery.values().stream()
.filter(stats ->
!_cancelSentQueries.contains(stats.getQueryId()))
.max(Comparator.comparing(AggregatedStats::getAllocatedBytes))
.orElse(null);
```
There are a few more changes that was required to safely add this feature.
`PerQueryCPUMemAccountantFactory.aggregate(boolean triggered)` performs two
functions:
* It reaps finished tasks and associated metadata - this is relevant to this
feature. `_cancelledSentQueries` entries also have to be reaped.
* If triggered, it also aggregates resource usage from `_threadEntriesMap`
to a `Map<String, AggregatedStats>`.
The dual functionality made it hard to write unit tests. So the function has
been split into:
* `reapFinishedTasks`
* `getQueryResources` - this already existed.
The main change is that `if (isTriggered) { ... }` blocks have been removed.
Another change to help with testing is that
`PerQueryCPUMemAccountant.WatcherTask.run()` has been extracted to
`PerQueryCPUMemAccountant.WatcherTask.runOnce()`. This helps to run one
iteration at a time in tests. Also it is possible to override this function in
a test to only focus on relevant operations.
There are other minor changes to change member visibility to `protected` and
new getter functions to also help with testing.
Consequently, the other major contribution of this PR is unit tests for
reap/aggregate functions as well as testing for duplicate cancel attempts.
Closes #16110
--
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]
