gianm commented on code in PR #19658:
URL: https://github.com/apache/druid/pull/19658#discussion_r3596333340
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java:
##########
@@ -137,6 +137,32 @@ public class SegmentLoaderConfig
private long combinedMaxSize = 0;
+ public SegmentLoaderConfig()
+ {
+ }
+
+ private SegmentLoaderConfig(SegmentLoaderConfig other)
+ {
+ this.locations = other.locations;
+ this.lazyLoadOnStart = other.lazyLoadOnStart;
+ this.deleteOnRemove = other.deleteOnRemove;
+ this.dropSegmentDelayMillis = other.dropSegmentDelayMillis;
+ this.announceIntervalMillis = other.announceIntervalMillis;
+ this.numLoadingThreads = other.numLoadingThreads;
+ this.numBootstrapThreads = other.numBootstrapThreads;
+ this.numThreadsToLoadSegmentsIntoPageCacheOnDownload =
other.numThreadsToLoadSegmentsIntoPageCacheOnDownload;
+ this.numThreadsToLoadSegmentsIntoPageCacheOnBootstrap =
other.numThreadsToLoadSegmentsIntoPageCacheOnBootstrap;
+ this.infoDir = other.infoDir;
+ this.statusQueueMaxSize = other.statusQueueMaxSize;
+ this.virtualStorage = other.virtualStorage;
+ this.virtualStorageLoadThreads = other.virtualStorageLoadThreads;
+ this.virtualStorageUseVirtualThreads =
other.virtualStorageUseVirtualThreads;
+ this.virtualStorageIsEphemeral = other.virtualStorageIsEphemeral;
+ this.virtualStorageMetadataReservationEstimate =
other.virtualStorageMetadataReservationEstimate;
+ this.virtualStoragePartialDownloadsEnabled =
other.virtualStoragePartialDownloadsEnabled;
+ this.combinedMaxSize = other.combinedMaxSize;
Review Comment:
+1, `virtualStorageCoalesceGapBytes` and `virtualStorageMaxFetchRunBytes`
are apparently missing.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/SegmentCacheManagerFactory.java:
##########
@@ -43,26 +47,52 @@ public class SegmentCacheManagerFactory
{
private final IndexIO indexIO;
private final ObjectMapper jsonMapper;
+ /**
+ * Resolved lazily (only on the {@code virtualStorage=true} path of {@link
#manufacturate}) so that injecting this
+ * factory into a component that only builds non-virtual caches (e.g. {@code
DruidInputSource}) does not force the
+ * process-wide {@link EphemeralStorageLoading} pool to be created on
processes that never do on-demand loading
+ * (Overlord, MiddleManager).
+ */
+ private final Provider<StorageLoadingThreadPool> loadingThreadPoolProvider;
@Inject
public SegmentCacheManagerFactory(
IndexIO indexIO,
- @Json ObjectMapper mapper
+ @Json ObjectMapper mapper,
+ @EphemeralStorageLoading Provider<StorageLoadingThreadPool>
loadingThreadPoolProvider
)
{
this.indexIO = indexIO;
this.jsonMapper = mapper;
+ this.loadingThreadPoolProvider = loadingThreadPoolProvider;
+ }
+
+ /**
+ * Creates a factory whose {@code virtualStorage=true} managers get their
own private loading pool of default size,
+ * instead of the process-wide, lifecycle-managed {@link
EphemeralStorageLoading} pool that Guice injects. Intended
+ * for tests and manual construction where that shared pool is not available
via injection; the created pool is not
+ * lifecycle-managed, which is fine for short-lived JVMs.
+ */
+ public static SegmentCacheManagerFactory createWithOwnedPool(IndexIO
indexIO, ObjectMapper mapper)
Review Comment:
IMO it's OK (and preferred) to remove the 2-arg constructor.
--
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]
