somandal commented on code in PR #15368:
URL: https://github.com/apache/pinot/pull/15368#discussion_r2019506948
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalanceSummaryResult.java:
##########
@@ -240,6 +240,92 @@ public Map<String, ServerSegmentChangeInfo>
getServerSegmentChangeInfo() {
}
}
+ public static class ConsumingSegmentSummaryPerServer {
+ private final int _numConsumingSegmentToBeAdded;
+ private final Integer _totalOffsetsNeedToCatchUp;
+
+ @JsonCreator
+ public ConsumingSegmentSummaryPerServer(
+ @JsonProperty("numConsumingSegmentToBeAdded") int
numConsumingSegmentToBeAdded,
+ @JsonProperty("totalOffsetsNeedToCatchUp") @Nullable Integer
totalOffsetsNeedToCatchUp) {
+ _numConsumingSegmentToBeAdded = numConsumingSegmentToBeAdded;
+ _totalOffsetsNeedToCatchUp = totalOffsetsNeedToCatchUp;
+ }
+
+ @JsonProperty
+ public int getNumConsumingSegmentToBeAdded() {
+ return _numConsumingSegmentToBeAdded;
+ }
+
+ @JsonProperty
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public Integer getTotalOffsetsNeedToCatchUp() {
+ return _totalOffsetsNeedToCatchUp;
+ }
+ }
+
+ public static class ConsumingSegmentToBeMovedSummary {
+ private final int _numConsumingSegmentsToBeMoved;
+ private final int _numServerGettingConsumingSegmentsAdded;
+ private final Map<String, Integer> _topConsumingSegmentsOffsetsToCatchUp;
+ private final Map<String, Integer>
_oldestConsumingSegmentsToBeMovedInMinutes;
Review Comment:
The reason why I'm insisting so much on clear naming is because often times
when people need to run rebalance they may not have time to go read detailed
docs or code to understand what each item means. Often people might be under
time crunch or pressure, especially during oncall etc.
I think having clear naming can help a lot with making the user experience
better for anyone that needs to run this, and this is a very common operation.
So I highly recommend removing ambiguity in naming for the JSON fields at the
very least.
If you cannot remove ambiguity in the name of the fields, can you add a
"description" String field in the JSON object that explains what the fields
mean (as recommended by @npawar) so that it is displayed when someone runs this
summary command.
I've not added "description" so far because the names were pretty self
explanatory so far. This consuming segment stuff is more involved and
complicated than the other parts of the summary, so definitely recommend adding
something here.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
