clintropolis commented on code in PR #19535:
URL: https://github.com/apache/druid/pull/19535#discussion_r3415427779


##########
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:
   agree this was a wack failure mode, I have reworked stuff so that if we 
detect partials on disk during startup and partials are not enabled we now 
delete them instead of exploding at query time. I have also switched the config 
to disable this partial load functionality until we have some more time to 
mature things.
   
   With regards to rollback to older versions, what happens is that on startup 
mount will fail with an exception because the path will not contain expected v9 
or v10 segment files, and the handling on that exception will trigger deletion 
because the path exists _unless_ `druid.segmentCache.deleteOnRemove` is false 
(it is true by default). We could do something kind of tricky to ensure that 
they are always deleted, which is _always_ write the 
`DOWNLOAD_START_MARKER_FILE_NAME` for partial segments so it would trigger the 
'corrupted segment' handling if we tried to mount as a regular segment, but I 
have not done that yet in this PR because it feels kind of strange.



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