[GitHub] himanshug edited a comment on issue #3956: Thread safe reads for aggregators in IncrementalIndex

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

himanshug edited a comment on issue #3956: Thread safe reads for aggregators in IncrementalIndex
URL: https://github.com/apache/incubator-druid/pull/3956#issuecomment-407502404
 
 
   In general, I agree with @leventov  here because different aggregators can handle concurrency with varying degree of efficiency.
   Unless, of course, there is a systematic way to do things that takes care of above e.g. introducing "boolean isThreadSafe()" method or something like that on Aggregator and then based on the answer, handle things correctly in IncrementalIndex. Then Aggregators can make the choice.
   Or else, I think aggregators not handling it properly are just buggy and should be fixed. Maybe update the aggregator doc with some blurbs on thread safety requirements.
   
   That said, we need synchronization only for realtime indexing code path and historical nodes pay the penalty of thread safety unnecessarily. If we could do something systematic to change the two code paths in some way that allows historicals not paying for thread safety, that would be good.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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: dev-unsubscribe@druid.apache.org
For additional commands, e-mail: dev-help@druid.apache.org