kfaraz commented on code in PR #20199:
URL: https://github.com/apache/druid/pull/20199#discussion_r3903714069
##########
server/src/main/java/org/apache/druid/server/coordinator/stats/Stats.java:
##########
@@ -106,12 +106,25 @@ public static class SegmentQueue
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
+ // planning figures and may exceed the physical disk measured by
STORAGE_CAPACITY and USED_STORAGE when virtual
+ // storage is in use.
public static final CoordinatorStat REQUIRED_CAPACITY
= CoordinatorStat.toDebugAndEmit("reqdCap", "tier/required/capacity");
+ public static final CoordinatorStat ASSIGNABLE_CAPACITY
+ = CoordinatorStat.toDebugAndEmit("assignableCap",
"tier/assignable/capacity");
+ // 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.
Review Comment:
Please convert this into a javadoc.
##########
server/src/test/java/org/apache/druid/server/coordinator/simulate/HistoricalTierAliasTest.java:
##########
@@ -104,6 +109,46 @@ public void testCapacityMetricsAreTaggedWithAlias()
);
}
+ /**
+ * {@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.
+ */
Review 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.
##########
server/src/main/java/org/apache/druid/server/coordinator/stats/Stats.java:
##########
@@ -106,12 +106,25 @@ public static class SegmentQueue
public static class Tier
{
+ // Assignment budget of a tier, denominated in full segment size.
REQUIRED_CAPACITY is the demand the rules
Review Comment:
Please convert to javadoc
##########
server/src/main/java/org/apache/druid/server/coordinator/stats/Stats.java:
##########
@@ -106,12 +106,25 @@ public static class SegmentQueue
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
Review Comment:
Please move the part about the assignable capacity into a javadoc for that
field itself.
--
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]
