unlimited number of off-heap buffers created from IntermediateProcessingPool

[himanshug]

Intermediate Processing Pool (powered by StupidPool ) has no limit on how many objects can be constructed. It is usually fine for historical nodes where concurrency is limited by druid.processing.numThreads .

However, at broker, when nested groupBy queries come.. they each need at least one buffer to process the outer query.
It is definitely not accounted in GroupBy-v1 implementation and even in GroupBy-v2 it does not seem to be taken care of.
As a result, when enough nested groupBy queries are sent to Broker, they result in creating enough off-heap "intermediate processing buffers" to bring the host to OOM and sometimes crashing the host.
( Above scenario happened with a user using GroupBy-v1 queries, however I haven't verified things yet. Comment about GroupBy-v2 having similar problem is purely a feel from quick code read in GroupByStrategyV2.prepareResources(..))

I am considering a configurable upper limit on "Intermediate Processing Pool", it would fail if more objects are asked out of it than configured upper limit. That would ensure that an overall limit is imposed irrespective of bugs or no limits in specific query implementations.
That would fail queries in the worst case but save host from going OOM and resulting outage.

[gianm]

It should be ok in groupBy v2 since v2 uses merge buffers, not processing buffers, and those come from a limited-size pool.

[himanshug]

@gianm thanks, good to know that, so one workaround available is to migrate to groupBy-v2 which might not be possible immediately in current user scenario for various reasons.

that said, even if things are fixed in query implementation ... it is good to have a hard limit inside intermediate-processing-pool itself just in case there were bugs in the code using intermediate-processing-pool.
I hope that is alright with you ?

[jihoonson]

it is good to have a hard limit inside intermediate-processing-pool itself just in case there were bugs in the code using intermediate-processing-pool.

BlockingPool is the limited-size pool and it might be better for this use case.

[gianm]

I think it would be fine to switch the intermediate pool to a BlockingPool (same as merge buffers)

[himanshug]

yeah, I'm ok with using BlockingPool to enable that too.

[stale]

This issue has been marked as stale due to 280 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the [email protected] list. Thank you for your contributions.

[stale]

This issue has been marked as stale due to 280 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the [email protected] list. Thank you for your contributions.

[stale]

This issue has been closed due to lack of activity. If you think that is incorrect, or the issue requires additional review, you can revive the issue at any time.