kfaraz commented on code in PR #14616:
URL: https://github.com/apache/druid/pull/14616#discussion_r1268397331
##########
server/src/main/java/org/apache/druid/server/coordination/SegmentLoadDropHandler.java:
##########
@@ -789,15 +789,20 @@ private CustomSettableFuture(
public void resolve()
{
- synchronized (statusRefs) {
+ synchronized (requestStatusesLock) {
Review Comment:
Thanks for calling it out. Yes, it is better if I just add a comment
explaining the logic.
I made the switch to make sure that the update invoked by
`requestStatuses.invalidate()` is safeguarded.
The current synchronization on `statusRefs` essentially meant that only one
thread can be executing the `resolve` of a given `CustomSettableFuture` at any
point. That condition still holds but with less concurrency, i.e. in the new
code, only one thread can be executing the `resolve` of _any_
`CustomSettableFuture` at any point.
So, in other words, while multiple segments might be loaded by the
historical in parallel, the futures will only be resolved serially.
This will not have any other impact on the correctness or performance of
this code.
--
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]
