feat: Add new tier/used/capacity metric to coordinator that reports actual loaded bytes across a tier - #20199
feat: Add new tier/used/capacity metric to coordinator that reports actual loaded bytes across a tier#20199capistrant wants to merge 10 commits into
tier/used/capacity metric to coordinator that reports actual loaded bytes across a tier#20199Conversation
kfaraz
left a comment
There was a problem hiding this comment.
Minor suggestion on naming.
| |`tier/replication/factor`|Configured maximum replication factor in each tier. The `tierAlias` dimension is emitted only when the tier belongs to an alias configured via [`historicalTierAliases`](../configuration/index.md#dynamic-configuration).|`tier`, `tierAlias`|Varies| | ||
| |`tier/required/capacity`|Total capacity in bytes required in each tier. The `tierAlias` dimension is emitted only when the tier belongs to an alias configured via [`historicalTierAliases`](../configuration/index.md#dynamic-configuration).|`tier`, `tierAlias`|Varies| | ||
| |`tier/total/capacity`|Total capacity in bytes available in each tier. The `tierAlias` dimension is emitted only when the tier belongs to an alias configured via [`historicalTierAliases`](../configuration/index.md#dynamic-configuration).|`tier`, `tierAlias`|Varies| | ||
| |`tier/total/capacity`|Total capacity in bytes available in each tier. Can be greater than physical disk capacity if using virtual storage. The `tierAlias` dimension is emitted only when the tier belongs to an alias configured via [`historicalTierAliases`](../configuration/index.md#dynamic-configuration).|`tier`, `tierAlias`|Varies| |
There was a problem hiding this comment.
With virtual storage, I feel total/capacity could now be ambiguous. Does this metric currently (before this PR) report the total virtual capacity or the total physical capacity?
Since we are revisiting these metrics, I feel we could distinguish them by naming them something like:
tier/storage/requiredtier/storage/physicalCapacitytier/storage/virtualCapacitytier/storage/used- and deprecated the existing
tier/total/capacitymetric.
There was a problem hiding this comment.
tier/storage/used I fully agree with. that is better name than I had, wil change.
I am considering something like tier/storage/assignableCapacity as the replacement for deprecated tier/total/capacity. It is what the coordinator sees as the amount of data it can assign to a tier.
then leaving tier/storage/capacity as is to reduce metric name churn?
thoughts?
There was a problem hiding this comment.
I guess I forgot about required in this calculus of trying to reduce churn
There was a problem hiding this comment.
ok my latest thought is
tier/required/capacity (unchanged) rule demand for full loads
tier/assignable/capacity (renamed) sum(maxSize) — assignment budget
tier/storage/capacity (newly documented) sum(storageSize) — real disk
tier/storage/used (new) sum(currSize) — real bytes on disk
There was a problem hiding this comment.
With the tier/storage, I was trying to ensure that they all have the same metric prefix.
But your suggested names would work too 👍🏻 , as long as it is easy to distinguish between the physical and the virtual.
kfaraz
left a comment
There was a problem hiding this comment.
Minor suggestions, changes look good.
| // Superseded by ASSIGNABLE_CAPACITY, which carries the same value under a name that says it is an assignment | ||
| // budget rather than physical disk. Still emitted so existing dashboards keep working; remove after deprecation. |
There was a problem hiding this comment.
Please convert this into a javadoc.
| public static class Tier | ||
| { | ||
| // Assignment budget of a tier, denominated in full segment size. REQUIRED_CAPACITY is the demand the rules | ||
| // place on the tier to full load all segments, ASSIGNABLE_CAPACITY the supply the tier advertises for it. Both are |
There was a problem hiding this comment.
Please move the part about the assignable capacity into a javadoc for that field itself.
|
|
||
| public static class Tier | ||
| { | ||
| // Assignment budget of a tier, denominated in full segment size. REQUIRED_CAPACITY is the demand the rules |
| /** | ||
| * {@code tier/storage/used} reports what the tier's historicals have announced as loaded, so it stays at zero | ||
| * until segments are actually on disk, unlike {@code tier/required/capacity} which reports what the rules demand. | ||
| */ |
There was a problem hiding this comment.
Nit: Try to remove this javadoc and incorporate what it does into the method name itself, and small 1-line comments inside the method, if needed.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 5 of 5 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
| public static final CoordinatorStat REQUIRED_CAPACITY | ||
| = CoordinatorStat.toDebugAndEmit("reqdCap", "tier/required/capacity"); | ||
| public static final CoordinatorStat ASSIGNABLE_CAPACITY | ||
| = CoordinatorStat.toDebugAndEmit("assignableCap", "tier/assignable/capacity"); |
There was a problem hiding this comment.
[P2] Register new tier metrics in default emitter maps
The new tier/assignable/capacity and tier/storage/used stats are absent from the bundled Prometheus, StatsD, and filtered logging metric maps, so default exporters omit these metrics even though the coordinator emits them. Add both metrics with their tier and tierAlias dimensions and gauge semantics, plus mapping coverage tests.
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 0 |
| Total | 1 |
Reviewed 7 of 7 changed files.
Validation: reverse git apply --check against the canonical diff passed. Builds and tests were not run.
This is an automated review by Codex GPT-5.6-Luna(max)
| "tier/replication/factor" : { "dimensions" : ["tier", "tierAlias"], "type" : "count", "help": "Configured maximum replication factor in each tier."}, | ||
| "tier/required/capacity" : { "dimensions" : ["tier", "tierAlias"], "type" : "count", "help": "Total capacity in bytes required in each tier."}, | ||
| "tier/total/capacity" : { "dimensions" : ["tier", "tierAlias"], "type" : "count", "help": "Total capacity in bytes available in each tier."}, | ||
| "tier/assignable/capacity": { "dimensions" : ["tier", "tierAlias"], "type" : "count", "help": "Total capacity in bytes available for assignment in each tier."}, |
There was a problem hiding this comment.
[P1] Register tier state metrics as gauges
The new tier/assignable/capacity, tier/storage/capacity, and tier/storage/used metrics are configured as Prometheus counters. PrometheusEmitter increments counters for every coordinator event, so these current-state values accumulate across cycles, never decrease when capacity or usage falls, and are exposed with counter semantics. Configure all three as gauge metrics.
Description
new metric
tier/storage/usedtier/storage/usedis a new coordinator metric that emits with atierand optionaltierAlias(if you enabled aliasing) dimension. This metric is helpful for monitoring the actually realized loaded bytes on a tier. Especially in a world of partial loads, where your tier may "require" a very high amount of storage to store everything on disk as a full load, but if you are using virtual storage and partial loads, you care more about the actual realized bytes on your historicals and may want to monitor this true consumption.renamed metric
tier/total/capacitytotier/assignable/capacityold metric will continue to be emitted until druid 40. The rename is being done because the value is not a true real capacity but rather assignable in that with virtual storage you can over assign to a tier because virtual storage will evict segments if it needs to load others.
document
tier/storage/capacitythis metric was added but not documented. it is the real physical capacity of the tier.
statsd and prometheus
the default metrics are updated to reflect the new metrics as well as emit the existing but missing
tier/storage/capacitywhich is the true physical size.Release note
New coordinator metric
tier/storage/usedthat helps track the actual bytes used per tier in a cluster. deprecatedtier/total/capacityand replaced it withtier/assignable/capacity. both will be emitted, but please move your monitors to the new metric before druid 40 releases.Key changed/added classes in this PR
PrepareBalancerAndLoadQueuesThis PR has: