This is an automated email from the ASF dual-hosted git repository.
gianm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 983c0c37db2 minor: Get StorageMonitor metrics from StorageLocations.
(#19494)
983c0c37db2 is described below
commit 983c0c37db2ca238fc763d8c3c6ec3dee89e1144
Author: Gian Merlino <[email protected]>
AuthorDate: Thu May 21 23:57:12 2026 -0700
minor: Get StorageMonitor metrics from StorageLocations. (#19494)
This patch changes StorageMonitor to get metrics from StorageLocations
directly, rather than through SegmentLocalCacheManager. This simplifies
the logic by removing an unnecessary layer. It also ensures that metrics
are reported properly no matter how the StorageLocations are accessed.
---
.../druid/msq/indexing/IndexerWorkerContext.java | 2 +-
.../org/apache/druid/guice/StorageNodeModule.java | 9 +--
.../druid/segment/loading/SegmentCacheManager.java | 5 +-
.../segment/loading/SegmentLocalCacheManager.java | 31 +----------
.../segment/loading/StorageLocationStats.java | 14 +++++
.../apache/druid/segment/loading/StorageStats.java | 64 ----------------------
.../loading/VirtualStorageLocationStats.java | 19 +++++++
.../druid/server/metrics/StorageMonitor.java | 34 +++++-------
.../segment/loading/NoopSegmentCacheManager.java | 8 +--
9 files changed, 58 insertions(+), 128 deletions(-)
diff --git
a/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/IndexerWorkerContext.java
b/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/IndexerWorkerContext.java
index 82d462fd096..b5058e05371 100644
---
a/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/IndexerWorkerContext.java
+++
b/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/IndexerWorkerContext.java
@@ -170,7 +170,7 @@ public class IndexerWorkerContext implements WorkerContext
injector.getInstance(SegmentCacheManagerFactory.class)
.manufacturate(new File(toolbox.getIndexingTmpDir(),
"segment-fetch"), true);
final SegmentManager segmentManager = new SegmentManager(cacheManager);
- final StorageMonitor storageMonitor = new StorageMonitor(cacheManager,
task::getMetricBuilder);
+ final StorageMonitor storageMonitor = new
StorageMonitor(cacheManager.getLocations(), task::getMetricBuilder);
toolbox.addMonitor(storageMonitor);
final ServiceClientFactory serviceClientFactory =
injector.getInstance(Key.get(ServiceClientFactory.class,
EscalatedGlobal.class));
diff --git a/server/src/main/java/org/apache/druid/guice/StorageNodeModule.java
b/server/src/main/java/org/apache/druid/guice/StorageNodeModule.java
index e030cd8cd2a..979dce5c3a6 100644
--- a/server/src/main/java/org/apache/druid/guice/StorageNodeModule.java
+++ b/server/src/main/java/org/apache/druid/guice/StorageNodeModule.java
@@ -21,7 +21,6 @@ package org.apache.druid.guice;
import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Binder;
-import com.google.inject.Injector;
import com.google.inject.Module;
import com.google.inject.Provides;
import com.google.inject.ProvisionException;
@@ -34,7 +33,6 @@ import org.apache.druid.java.util.emitter.EmittingLogger;
import org.apache.druid.query.DruidProcessingConfig;
import org.apache.druid.segment.DefaultColumnFormatConfig;
import org.apache.druid.segment.column.ColumnConfig;
-import org.apache.druid.segment.loading.SegmentCacheManager;
import org.apache.druid.segment.loading.SegmentLoaderConfig;
import org.apache.druid.segment.loading.StorageLocation;
import org.apache.druid.segment.loading.StorageLocationSelectorStrategy;
@@ -143,12 +141,9 @@ public class StorageNodeModule implements Module
@Provides
@LazySingleton
- @Nullable
- public StorageMonitor provideStorageMonitor(
- Injector injector
- )
+ public StorageMonitor provideStorageMonitor(List<StorageLocation> locations)
{
- return new StorageMonitor(injector.getInstance(SegmentCacheManager.class),
null);
+ return new StorageMonitor(locations, null);
}
/**
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/SegmentCacheManager.java
b/server/src/main/java/org/apache/druid/segment/loading/SegmentCacheManager.java
index 922f0f17b0a..393b0ac8831 100644
---
a/server/src/main/java/org/apache/druid/segment/loading/SegmentCacheManager.java
+++
b/server/src/main/java/org/apache/druid/segment/loading/SegmentCacheManager.java
@@ -141,8 +141,7 @@ public interface SegmentCacheManager
void shutdown();
/**
- * Collect {@link StorageStats}, if available.
+ * Returns the storage locations backing this cache manager.
*/
- @Nullable
- StorageStats getStorageStats();
+ List<StorageLocation> getLocations();
}
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
index 8514c4e92a8..5372baf4c24 100644
---
a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
+++
b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java
@@ -53,10 +53,8 @@ import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
-import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
@@ -704,39 +702,14 @@ public class SegmentLocalCacheManager implements
SegmentCacheManager
}
}
- @Nullable
- @Override
- public StorageStats getStorageStats()
- {
- if (config.isVirtualStorage()) {
- final Map<String, VirtualStorageLocationStats> locationStats = new
HashMap<>();
- for (StorageLocation location : locations) {
- locationStats.put(location.getPath().toString(),
location.resetWeakStats());
- }
- return new StorageStats(
- Map.of(),
- locationStats
- );
- } else {
- final Map<String, StorageLocationStats> locationStats = new HashMap<>();
- for (StorageLocation location : locations) {
- locationStats.put(location.getPath().toString(),
location.resetStaticStats());
- }
- return new StorageStats(
- locationStats,
- Map.of()
- );
- }
- }
-
@VisibleForTesting
public ConcurrentHashMap<DataSegment, ReferenceCountingLock>
getSegmentLocks()
{
return segmentLocks;
}
- @VisibleForTesting
- List<StorageLocation> getLocations()
+ @Override
+ public List<StorageLocation> getLocations()
{
return locations;
}
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/StorageLocationStats.java
b/server/src/main/java/org/apache/druid/segment/loading/StorageLocationStats.java
index da27aedba0b..2600c0e1fee 100644
---
a/server/src/main/java/org/apache/druid/segment/loading/StorageLocationStats.java
+++
b/server/src/main/java/org/apache/druid/segment/loading/StorageLocationStats.java
@@ -58,4 +58,18 @@ public interface StorageLocationStats
* Number of bytes dropped during the measurement period
*/
long getDropBytes();
+
+ /**
+ * Whether any stats are nonzero.
+ */
+ default boolean hasStats()
+ {
+ return getUsedBytes() != 0
+ || getLoadBeginCount() != 0
+ || getLoadBeginBytes() != 0
+ || getLoadCount() != 0
+ || getLoadBytes() != 0
+ || getDropCount() != 0
+ || getDropBytes() != 0;
+ }
}
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/StorageStats.java
b/server/src/main/java/org/apache/druid/segment/loading/StorageStats.java
deleted file mode 100644
index bfd4dc65e2b..00000000000
--- a/server/src/main/java/org/apache/druid/segment/loading/StorageStats.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.apache.druid.segment.loading;
-
-import org.apache.druid.guice.annotations.UnstableApi;
-
-import java.util.Map;
-
-/**
- * Collection of {@link StorageLocationStats} and {@link
VirtualStorageLocationStats} for all storage locations within
- * a {@link SegmentCacheManager} so that {@link
SegmentCacheManager#getStorageStats()} can be used by
- * {@link org.apache.druid.server.metrics.StorageMonitor} to track segment
cache activity.
- * <p>
- * Note that the stats are not tied explicitly to the {@link StorageLocation}
implementation used by
- * {@link SegmentLocalCacheManager}, but it does implement this stuff.
- */
-@UnstableApi
-public class StorageStats
-{
- private final Map<String, StorageLocationStats> stats;
- private final Map<String, VirtualStorageLocationStats> virtualStats;
-
- public StorageStats(
- final Map<String, StorageLocationStats> stats,
- final Map<String, VirtualStorageLocationStats> virtualStats
- )
- {
- this.stats = stats;
- this.virtualStats = virtualStats;
- }
-
- /**
- * Map of location label (such as file path) to {@link StorageLocationStats}
- */
- public Map<String, StorageLocationStats> getLocationStats()
- {
- return stats;
- }
-
- /**
- * Map of location label (such as file path) to {@link
VirtualStorageLocationStats}
- */
- public Map<String, VirtualStorageLocationStats> getVirtualLocationStats()
- {
- return virtualStats;
- }
-}
diff --git
a/server/src/main/java/org/apache/druid/segment/loading/VirtualStorageLocationStats.java
b/server/src/main/java/org/apache/druid/segment/loading/VirtualStorageLocationStats.java
index d7d5dfcfa3f..5f63b08b659 100644
---
a/server/src/main/java/org/apache/druid/segment/loading/VirtualStorageLocationStats.java
+++
b/server/src/main/java/org/apache/druid/segment/loading/VirtualStorageLocationStats.java
@@ -86,4 +86,23 @@ public interface VirtualStorageLocationStats
* Number of operations which could not be loaded due to insufficient space
during the measurement period
*/
long getRejectCount();
+
+ /**
+ * Whether any stats are nonzero.
+ */
+ default boolean hasStats()
+ {
+ return getUsedBytes() != 0
+ || getHoldCount() != 0
+ || getHoldBytes() != 0
+ || getHitCount() != 0
+ || getHitBytes() != 0
+ || getLoadBeginCount() != 0
+ || getLoadBeginBytes() != 0
+ || getLoadCount() != 0
+ || getLoadBytes() != 0
+ || getEvictionCount() != 0
+ || getEvictionBytes() != 0
+ || getRejectCount() != 0;
+ }
}
diff --git
a/server/src/main/java/org/apache/druid/server/metrics/StorageMonitor.java
b/server/src/main/java/org/apache/druid/server/metrics/StorageMonitor.java
index 26bae751568..a0a551d8bb1 100644
--- a/server/src/main/java/org/apache/druid/server/metrics/StorageMonitor.java
+++ b/server/src/main/java/org/apache/druid/server/metrics/StorageMonitor.java
@@ -25,16 +25,15 @@ import org.apache.druid.guice.annotations.LoadScope;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
import org.apache.druid.java.util.metrics.AbstractMonitor;
-import org.apache.druid.segment.loading.SegmentCacheManager;
+import org.apache.druid.segment.loading.StorageLocation;
import org.apache.druid.segment.loading.StorageLocationStats;
-import org.apache.druid.segment.loading.StorageStats;
import org.apache.druid.segment.loading.VirtualStorageLocationStats;
import javax.annotation.Nullable;
-import java.util.Map;
+import java.util.List;
/**
- * Monitor to emit output of {@link SegmentCacheManager#getStorageStats()}
+ * Monitor to emit stats from {@link StorageLocation}.
*/
@LoadScope(roles = {
NodeRole.BROKER_JSON_NAME,
@@ -150,41 +149,38 @@ public class StorageMonitor extends AbstractMonitor
*/
public static final String VSF_REJECT_COUNT = "storage/virtual/reject/count";
- private final SegmentCacheManager cacheManager;
+ private final List<StorageLocation> locations;
private final Supplier<ServiceMetricEvent.Builder> builderSupplier;
public StorageMonitor(
- SegmentCacheManager cacheManager,
+ List<StorageLocation> locations,
@Nullable Supplier<ServiceMetricEvent.Builder> builderSupplier
)
{
- this.cacheManager = cacheManager;
+ this.locations = locations;
this.builderSupplier = builderSupplier == null ?
ServiceMetricEvent.Builder::new : builderSupplier;
}
@Override
public boolean doMonitor(ServiceEmitter emitter)
{
- final StorageStats stats = cacheManager.getStorageStats();
+ for (StorageLocation location : locations) {
+ final String label = location.getPath().toString();
- if (stats != null) {
- for (Map.Entry<String, StorageLocationStats> location :
stats.getLocationStats().entrySet()) {
- final StorageLocationStats staticStats = location.getValue();
- final ServiceMetricEvent.Builder builder = builderSupplier.get()
-
.setDimension(LOCATION_DIMENSION, location.getKey());
+ final StorageLocationStats staticStats = location.resetStaticStats();
+ final ServiceMetricEvent.Builder builder =
builderSupplier.get().setDimension(LOCATION_DIMENSION, label);
+ if (staticStats.hasStats()) {
emitter.emit(builder.setMetric(USED_BYTES,
staticStats.getUsedBytes()));
- emitter.emit(builder.setMetric(LOAD_BEGIN_COUNT,
staticStats.getLoadBeginCount()));
- emitter.emit(builder.setMetric(LOAD_BEGIN_BYTES,
staticStats.getLoadBeginBytes()));
emitter.emit(builder.setMetric(LOAD_COUNT,
staticStats.getLoadCount()));
emitter.emit(builder.setMetric(LOAD_BYTES,
staticStats.getLoadBytes()));
+ emitter.emit(builder.setMetric(LOAD_BEGIN_COUNT,
staticStats.getLoadBeginCount()));
+ emitter.emit(builder.setMetric(LOAD_BEGIN_BYTES,
staticStats.getLoadBeginBytes()));
emitter.emit(builder.setMetric(DROP_COUNT,
staticStats.getDropCount()));
emitter.emit(builder.setMetric(DROP_BYTES,
staticStats.getDropBytes()));
}
- for (Map.Entry<String, VirtualStorageLocationStats> location :
stats.getVirtualLocationStats().entrySet()) {
- final VirtualStorageLocationStats weakStats = location.getValue();
- final ServiceMetricEvent.Builder builder = builderSupplier.get()
-
.setDimension(LOCATION_DIMENSION, location.getKey());
+ final VirtualStorageLocationStats weakStats = location.resetWeakStats();
+ if (weakStats.hasStats()) {
emitter.emit(builder.setMetric(VSF_USED_BYTES,
weakStats.getUsedBytes()));
emitter.emit(builder.setMetric(VSF_HOLD_COUNT,
weakStats.getHoldCount()));
emitter.emit(builder.setMetric(VSF_HOLD_BYTES,
weakStats.getHoldBytes()));
diff --git
a/server/src/test/java/org/apache/druid/segment/loading/NoopSegmentCacheManager.java
b/server/src/test/java/org/apache/druid/segment/loading/NoopSegmentCacheManager.java
index bec367ddc9a..ae118afa7bb 100644
---
a/server/src/test/java/org/apache/druid/segment/loading/NoopSegmentCacheManager.java
+++
b/server/src/test/java/org/apache/druid/segment/loading/NoopSegmentCacheManager.java
@@ -24,7 +24,6 @@ import org.apache.druid.segment.SegmentLazyLoadFailCallback;
import org.apache.druid.timeline.DataSegment;
import org.apache.druid.timeline.SegmentId;
-import javax.annotation.Nullable;
import java.io.File;
import java.util.List;
import java.util.Optional;
@@ -48,7 +47,7 @@ public class NoopSegmentCacheManager implements
SegmentCacheManager
}
@Override
- public boolean canLoadSegmentOnDemand(DataSegment segment)
+ public boolean canLoadSegmentOnDemand(DataSegment dataSegment)
{
return false;
}
@@ -119,10 +118,9 @@ public class NoopSegmentCacheManager implements
SegmentCacheManager
throw new UnsupportedOperationException();
}
- @Nullable
@Override
- public StorageStats getStorageStats()
+ public List<StorageLocation> getLocations()
{
- return null;
+ return List.of();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
