capistrant commented on code in PR #19843:
URL: https://github.com/apache/druid/pull/19843#discussion_r3707088050
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -1338,15 +1338,23 @@ public DataSegment load(final DataSegment dataSegment)
throws SegmentLoadingExce
return loadPartial(dataSegment);
}
// virtual storage doesn't do anything with loading immediately, but
check to see if the segment is already cached
- // and if so, clear out the onUnmount action
+ // and if so, clear out the onUnmount action. An unwrapped request for a
segment currently held under a
+ // partial-load rule is the coordinator asking for the whole segment
again, so release the rule as well.
+ final boolean isFullLoadRequest =
!PartialLoadSpec.detectPartialLoadSpec(dataSegment.getLoadSpec());
final ReferenceCountingLock lock = lock(dataSegment);
synchronized (lock) {
try {
final SegmentCacheEntryIdentifier cacheEntryIdentifier = new
SegmentCacheEntryIdentifier(dataSegment.getId());
for (StorageLocation location : locations) {
final SegmentCacheEntry cacheEntry =
location.getCacheEntry(cacheEntryIdentifier);
- if (cacheEntry != null) {
- cacheEntry.setOnUnmount(null);
+ if (cacheEntry == null) {
+ continue;
+ }
+ cacheEntry.setOnUnmount(null);
+ if (isFullLoadRequest
Review Comment:
good point, after analysis, I think it is at best a waste of time. dropping
--
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]
