[GitHub] [incubator-druid] leventov opened a new issue #8886: IncrementalIndex's `in` ThreadLocal doesn't need to be instance-level

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

leventov opened a new issue #8886: IncrementalIndex's `in` ThreadLocal doesn't need to be instance-level
URL: https://github.com/apache/incubator-druid/issues/8886
 
 
   `IncrementalIndex`'s `ThreadLocal in` can be `static final`. This `ThreadLocal` doesn't store anything *specific to a given `IncrementalIndex`*: rather, it's used to set a `Row` temporarily, and there is no situation when an `IncrementalIndex` could call recursively into another `IncrementalIndex`, so this temporary assignment cannot be disturbed.
   
   As a safety net, we could add a check `if (in.get() != null) throw ISE(..); in.set(row);`
   
   Furthermore, it would be ideal to get rid of `ThreadLocal` altogether (which would be beneficial for performance, because every `Aggregator` wouldn't need to query `ThreadLocal` upon every indexed row (`ThreadLocal.get()` is like `Map.get()`, not super cheap), and for reducing complexity, but this requires a significant refactoring of `Aggregator` interface and/or indexing logic, so saving this idea for a better time.
   

----------------------------------------------------------------
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