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



##########
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:
       @cheddar , for the coordinator assignment algorithm that you noted, im 
confused how it protects against a segment never being loaded properly in the 
face of the bug that this pr is fixing. The coordinator will likely notice that 
a particular segment that was asked to be loaded hasnt been loaded, so it may 
request that the historical load the segment again, but if the historical still 
has the cache entry for an earlier successful load request for the same 
segment, how would it ever load the segment? And in the worse case, if the 
segment has at one point in time been loaded by all historicals in deployment, 
and none of them have been rebooted since, I believe that no historical will 
load the segment no matter how many times the coordinator requests them to do 
so, until they are restarted, and that cache is cleared. Please correct any 
misunderstandings I may have here.




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