kfaraz commented on PR #14643: URL: https://github.com/apache/druid/pull/14643#issuecomment-1648922131
> I feel like it's simpler to just leave the synchronized block in for run/joinAsync @georgew5656 , if you are comfortable with it, then we can proceed. 👍🏻 > i think this is a safer solution for druid 27 and we can maybe investigate this change to remove synchronization entirely after doing some more scale testing to see how much it helps. This is not a blocker for Druid 27 as the bug is in a contrib extension. But yes, it would be better to have it addressed. > we can maybe investigate this change to remove synchronization entirely after doing some more scale testing to see how much it helps. Removing this synchronization might not help a lot in performance as the critical section is very small and threads would not be blocked for long. The primary reason I did not prefer it was to have homogeneity in the code. It gets confusing to have synchronization in some places and not in the other places. And if we have synchronization in all places, then it does start affecting performance. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
