clintropolis commented on code in PR #18871:
URL: https://github.com/apache/druid/pull/18871#discussion_r2656538219
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java:
##########
@@ -77,6 +77,9 @@ public class SegmentLoaderConfig
@JsonProperty("virtualStorageLoadThreads")
private int virtualStorageLoadThreads = 2 *
runtimeInfo.getAvailableProcessors();
+ @JsonProperty("virtualStorageFabricEvictImmediately")
+ private boolean virtualStorageFabricEvictImmediately = false;
Review Comment:
i wonder if this should this just be set automatically by a module or
something instead of a user facing config that needs to be set manually, but
this is fine for now we can worry about that later
##########
server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java:
##########
@@ -444,6 +435,52 @@ public void release(CacheEntry entry)
}
}
+ /**
+ * Creates a release runnable for a {@link WeakCacheEntry} that handles
immediate eviction when configured.
+ * If {@link #evictImmediately} is true and there are no more holds after
releasing, the entry is immediately
+ * evicted from the cache. For new entries (isNewEntry=true), unmounted
entries are also removed.
+ */
+ private Runnable createWeakEntryReleaseRunnable(
+ final WeakCacheEntry weakEntry,
+ final boolean isNewEntry
+ )
+ {
+ return () -> {
+ weakEntry.release();
+
+ if (!isNewEntry && !evictImmediately) {
+ // No need to consider removal from weakCacheEntries on hold release.
+ return;
+ }
+
+ lock.writeLock().lock();
+ try {
+ weakCacheEntries.computeIfPresent(
+ weakEntry.cacheEntry.getId(),
+ (cacheEntryIdentifier, weakCacheEntry) -> {
+ // If we never successfully mounted, go ahead and remove so we
don't have a dead entry.
+ // Futhermore, if evictImmediately is set, evict on release if
all holds are gone.
+ final boolean isMounted = weakCacheEntry.cacheEntry.isMounted();
+ if ((isNewEntry && !isMounted)
+ || (evictImmediately && !weakCacheEntry.isHeld())) {
Review Comment:
i guess the implication here is that segments can only be used once per
download (or like, concurrently with multiple holds) if `evictImmediately` is
set, this feels worth a comment somewhere (or rebranding slightly like
suggested in other comment)
##########
server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java:
##########
@@ -106,9 +106,7 @@ public class StorageLocation
@GuardedBy("lock")
private WeakCacheEntry hand;
- /**
- * Current total size of files in bytes, including weak entries.
- */
+ private volatile boolean evictImmediately = false;
Review Comment:
nit: maybe call this `evictImmediatelyOnHoldRelease` or like..
`burnAfterReading` or something to make it more obvious what the behavior is?
--
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]
