FrankChen021 commented on code in PR #19739:
URL: https://github.com/apache/druid/pull/19739#discussion_r3650318921
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/SegmentCacheManagerFactory.java:
##########
@@ -88,24 +97,33 @@ 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
);
final SegmentLoaderConfig loaderConfig =
- new SegmentLoaderConfig()
+ segmentLoaderConfig.withVirtualStorage(virtualStorage)
Review Comment:
[P2] Preserve only compatible virtual-storage settings
Copying the entire injected `SegmentLoaderConfig` also inherits
`numThreadsToLoadSegmentsIntoPageCacheOnDownload` and
`numThreadsToLoadSegmentsIntoPageCacheOnBootstrap`. If either valid operator
setting is nonzero, every `virtualStorage=true` task cache now throws during
`SegmentLocalCacheManager` construction because virtual storage rejects
page-cache loading threads. Previously task caches started from defaults. Copy
only the intended virtual-storage tuning fields or explicitly reset the
incompatible settings.
--
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]
