kfaraz commented on a change in pull request #11717:
URL: https://github.com/apache/druid/pull/11717#discussion_r710310026



##########
File path: 
server/src/main/java/org/apache/druid/server/coordination/SegmentLoadDropHandler.java
##########
@@ -560,6 +560,11 @@ public void removeSegment(DataSegment segment, 
DataSegmentChangeCallback callbac
             },
             this::resolveWaitingFutures
         );
+      } else if (status.get().getState() == Status.STATE.SUCCESS) {

Review comment:
       I feel a better approach could be to augment the possible states to 
denote that the last success was a Drop/Load success rather than invalidating 
here. (e.g. Maybe we could add a new state SUCCESS_DROPPED, etc.)
   
   The current approach could cause a problem if we send three subsequent 
requests, each to Load the same segment.
   - Request 1 would honor the request and mark it as PENDING. The request 
would then complete and be marked SUCCESS.
   - Request 2 would invalidate the status of the previous one.
   - Request 3 would again try to Load the Segment, even though it's already 
loaded.
   
   Please correct me if my understanding is not right.




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

Reply via email to