[GitHub] [incubator-druid] leventov commented on issue #8884: Get rid of instance-level ThreadLocal in ConcurrentGrouper

[GitBox <gi...@apache.org>]

leventov commented on issue #8884: Get rid of instance-level ThreadLocal in ConcurrentGrouper
URL: https://github.com/apache/incubator-druid/issues/8884#issuecomment-554774347
 
 
   > It seems error-prone, but may perform better enough than the instance-level threadlocals that it could be worth it.
   
   I agree that it's not super elegant, but it at least seems less error-prone than the current solution which relies on the fact that `ConcurrentGrouper.aggregate()` is called from a very specific number of threads, and no more. `ThreadLocal` per se doesn't add any protection against any kind of error in this regard. Even when passed `threadNumber`, `ConcurrentGrouper` will anyway synchronize on the corresponding sub-grouper (as it also does now), because there is no synchronization point when they turn spilling.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org