clintropolis commented on a change in pull request #11717:
URL: https://github.com/apache/druid/pull/11717#discussion_r723833447
##########
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 wonder if the cache should be keyed by `DataSegment` instead of
`DataSegmentChangeRequest`, maybe something like `Cache<DataSegment,
AtomicReference<NonnullPair<DataSegmentChangeRequest, Status>>>`, so then you
could check if the change request is the same type of change. A different type
of change for the same segment would replace the old status so a reload would
not be incorrectly cached. `DataSegmentChangeRequest` itself would probably
need modified to have a new method:
```
@Nullable
default DataSegment getSegment()
{
return null;
}
```
this would be a bit more disruptive of a change though, also I haven't fully
thought it through so maybe there is some reason this wouldn't be good..
--
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]
