FrankChen021 commented on code in PR #19739:
URL: https://github.com/apache/druid/pull/19739#discussion_r3665455976
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/SegmentCacheManagerFactory.java:
##########
@@ -88,24 +96,39 @@ public static SegmentCacheManagerFactory
createWithOwnedPool(IndexIO indexIO, Ob
* {@link EphemeralStorageLoading} loading pool (shared across all per-task
caches and stopped by the lifecycle)
* rather than creating its own.
*
- * @param storageDir storage location
- * @param maxSize size limit, or null for no limit
- * @param virtualStorage whether to configure the cache manager in ephemeral
virtual storage mode. In this mode,
- * loading is triggered by {@link
SegmentCacheManager#acquireSegment(DataSegment, AcquireMode)},
- * and segment files are deleted as soon as all holds
are closed.
+ * @param storageDir storage location
+ * @param maxSize size limit, or null for no limit
+ * @param virtualStorage whether to configure the cache manager in
ephemeral virtual storage mode. In this
+ * mode, loading is triggered by
+ * {@link
SegmentCacheManager#acquireSegment(DataSegment, AcquireMode)}, and segment
+ * files are deleted as soon as all holds are
closed.
+ * @param partialDownloadsEnabled when true (and {@code virtualStorage} is
true), partial-eligible segments are read
+ * via on-demand column downloads rather than
downloaded in full up front. Has no
+ * effect when {@code virtualStorage} is
false.
*/
- public SegmentCacheManager manufacturate(File storageDir, Long maxSize,
boolean virtualStorage)
+ public SegmentCacheManager manufacturate(
+ File storageDir,
+ Long maxSize,
+ boolean virtualStorage,
+ boolean partialDownloadsEnabled
+ )
{
final StorageLocationConfig locationConfig = new StorageLocationConfig(
storageDir,
maxSize != null ? maxSize : Long.MAX_VALUE,
null
);
+ // For virtual storage, derive an ephemeral virtual-storage cache config
from the injected node config: this keeps
+ // its virtual-storage tuning while dropping the classic on-disk-cache
settings that don't apply to a per-task
+ // cache. Otherwise, start from the node config as-is. Then set the
per-task location and mode flags.
final SegmentLoaderConfig loaderConfig =
- new SegmentLoaderConfig()
- .setLocations(Collections.singletonList(locationConfig))
- .setVirtualStorage(virtualStorage)
- .setVirtualStorageIsEphemeral(virtualStorage);
+ (virtualStorage ? segmentLoaderConfig.toEphemeralVirtualStorage() :
segmentLoaderConfig)
Review Comment:
[P2] Keep node cache settings out of nonvirtual input caches
When `virtualStorage` is false, the production caller is `DruidInputSource`,
but this branch now clones the process-wide `druid.segmentCache` configuration
instead of the fresh defaults used before this PR. With
`numThreadsToLoadSegmentsIntoPageCacheOnDownload > 0`, every reader creates and
starts its own fixed executor; the input entity only drops segments and never
calls `shutdown()`, so a persistent Indexer leaks a pool per reader/task.
Inheriting `deleteOnRemove=false` also leaves downloaded input segments behind.
Derive this branch from defaults or reset these transient cache settings; only
virtual managers need the injected tuning.
--
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]
