eagerly allocate the intermediate computation buffers

[himanshug]

for better predictability and more importantly sometimes allocation may take significant amount of times (~ minute) resulting in very unpredictable query performance.

[gianm]

👍

[fjy]

👍

[leventov]

If intermediate buffers are effectively used in thread-local manner (what getNumThreads() as the number of pre-allocated buffers suggests), it should probably be rewritten with ThreadLocal<ByteBuffer>.

[himanshug]

@leventov that makes sense for the specific context of processing threads and i can imaging things being adjusted that way. however StupidPool is used in other places too where same thread might ask for multiple different buffers. my intention with this PR is to solve the problem mentioned in the description.

[cheddar]

Actually, using it as a ThreadLocal is also incorrect because this isn't something that actually follows the lifecycle of the threads. If one of the processing threads dies for some unknown reason, that doesn't necessarily mean that we want new objects to be created.

The reason to use ThreadLocal is when an object is necessarily tied to the lifecycle of a thread or if the cost of management of those objects is sufficiently small that you don't care about spurious instantiations. Neither of those statements are true in this case.

[leventov]

@cheddar ThreadLocal isn't necessarily tied only with a Thread, it could be tied with a pair of a Thread and some object (i. e. ThreadLocal field is not static).

I'm pretty sure StupidPool is overused in the Druid codebase and could be avoided somewhere. Recently we have issues with GC and OOMs in production which most likely come from StupidPool's "finalized" direct buffers. The issue is probably mitigated by #3629 and #3631, but I'm not sure is gone entirely.

[gianm]

@himanshug what did you want included in the release notes for this? (you tagged it "Release Notes")

[himanshug]

@gianm i wanted to mention just a note that don't set druid.processing.numThreads and druid.processing.buffer.sizeBytes to arbitrarily large values or historical/peon startup might fail to start.

added it under "Other Notes"