gianm commented on code in PR #19535:
URL: https://github.com/apache/druid/pull/19535#discussion_r3343345086
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -536,16 +897,30 @@ private AcquireSegmentAction
acquireExistingSegment(SegmentCacheEntryIdentifier
location.addWeakReservationHoldIfExists(identifier)
);
if (hold != null) {
- if (hold.getEntry().isMounted()) {
+ if (!(hold.getEntry() instanceof CompleteSegmentCacheEntry
complete)) {
+ // The eager (complete) acquire path found a non-complete entry
under this id. This only arises if
+ // partial-load on-disk state survived a toggle of
druid.segmentCache.virtualStoragePartialDownloadsEnabled
+ // to false (getCachedSegments reserves an on-disk partial layout
regardless of the flag). The eager path
+ // cannot serve a partial layout; surface a clear operator error
rather than a ClassCastException.
+ throw DruidException.forPersona(DruidException.Persona.OPERATOR)
Review Comment:
Two concerns:
1. Won't this also be an issue on downgrade?
2. Is it possible to do something that would cause the old version to repair
this rather than throwing an error? As is, it will be difficult to turn off
partial downloads once they are enabled.
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java:
##########
@@ -536,16 +897,30 @@ private AcquireSegmentAction
acquireExistingSegment(SegmentCacheEntryIdentifier
location.addWeakReservationHoldIfExists(identifier)
);
if (hold != null) {
- if (hold.getEntry().isMounted()) {
+ if (!(hold.getEntry() instanceof CompleteSegmentCacheEntry
complete)) {
+ // The eager (complete) acquire path found a non-complete entry
under this id. This only arises if
+ // partial-load on-disk state survived a toggle of
druid.segmentCache.virtualStoragePartialDownloadsEnabled
+ // to false (getCachedSegments reserves an on-disk partial layout
regardless of the flag). The eager path
+ // cannot serve a partial layout; surface a clear operator error
rather than a ClassCastException.
+ throw DruidException.forPersona(DruidException.Persona.OPERATOR)
Review Comment:
Two concerns:
1. Won't this also be an issue on downgrade?
2. Is it possible to repair this rather than throwing an error? As is, it
will be difficult to turn off partial downloads once they are enabled.
--
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]
