FrankChen021 commented on code in PR #20199:
URL: https://github.com/apache/druid/pull/20199#discussion_r3979213226
##########
server/src/main/java/org/apache/druid/server/coordinator/stats/Stats.java:
##########
@@ -106,12 +106,52 @@ public static class SegmentQueue
public static class Tier
{
+ // Segment storage of a tier, denominated in full segment size. These are
the assignment ledger: REQUIRED_STORAGE
+ // is the demand the rules place on the tier, STORAGE_CAPACITY the supply
it advertises. Under virtual storage a
+ // tier can address more storage than it has disk, so both may exceed
CACHE_CAPACITY.
+
+ /**
+ * The demand the rules place on the tier to full load all segments.
+ */
+ public static final CoordinatorStat REQUIRED_STORAGE
Review Comment:
[P2] Add new tier metrics to the filtered logging allowlist
`CoordinatorDutyGroup` emits these stats as `ServiceMetricEvent`s, but the
bundled `processing/src/main/resources/loggingEmitterAllowedMetrics.json` (and
its test copy) still contains only the old tier keys. With
`druid.emitter.logging.shouldFilterMetrics=true`, `LoggingEmitter` drops all
four new names (`tier/storage/required`, `tier/storage/capacity`,
`tier/segmentCache/capacity`, and `tier/segmentCache/used`) because it checks
exact metric-name membership. Add the new names to the bundled allowlist and
test fixture so the metrics are observable under the default filtered logger.
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/PrepareBalancerAndLoadQueues.java:
##########
@@ -184,20 +184,27 @@ private void collectHistoricalStats(
: RowKey.with(Dimension.TIER,
tier).and(Dimension.TIER_ALIAS, alias);
stats.add(Stats.Tier.HISTORICAL_COUNT, rowKey, historicals.size());
- long totalCapacity = 0;
long storageCapacity = 0;
+ long cacheCapacity = 0;
+ long cacheUsed = 0;
long cloneCount = 0;
for (ServerHolder holder : historicals) {
if (holder.isUnmanaged()) {
cloneCount += 1;
} else {
- totalCapacity += holder.getMaxSize();
- storageCapacity += holder.getStorageSize();
+ storageCapacity += holder.getMaxSize();
Review Comment:
[P1] Preserve the physical meaning of tier/storage/capacity
`tier/storage/capacity` already reports the physical segment-cache capacity
(`holder.getStorageSize()`) in the base branch, and it was added for VSF disk
monitoring. This change feeds the unchanged metric name with
`holder.getMaxSize()` instead. In virtual-storage mode `maxSize` can be far
larger than physical storage, so existing dashboards and alerts silently
under-report disk utilization. Keep this metric's existing physical semantics
and introduce a genuinely new name for assignable/virtual capacity, or preserve
the old physical value under a compatibility metric before renaming.
--
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]
