clintropolis commented on a change in pull request #10724:
URL: https://github.com/apache/druid/pull/10724#discussion_r553119211
##########
File path:
server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java
##########
@@ -87,6 +88,8 @@
private final int maxSegmentsInNodeLoadingQueue;
private final boolean pauseCoordination;
+ private static final EmittingLogger log = new
EmittingLogger(CoordinatorDynamicConfig.class);
Review comment:
nit: I don't think this needs to be `EmittingLogger` since it isn't
doing an alert anywhere
##########
File path:
server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java
##########
@@ -96,7 +99,7 @@ public CoordinatorDynamicConfig(
@JsonProperty("mergeBytesLimit") long mergeBytesLimit,
@JsonProperty("mergeSegmentsLimit") int mergeSegmentsLimit,
@JsonProperty("maxSegmentsToMove") int maxSegmentsToMove,
- @JsonProperty("percentOfSegmentsToConsiderPerMove") double
percentOfSegmentsToConsiderPerMove,
+ @JsonProperty("percentOfSegmentsToConsiderPerMove") Double
percentOfSegmentsToConsiderPerMove,
Review comment:
I guess the alternative way to fix this instead of switching out the
primitive would be to treat `0` as not configured and so use the default of
`100`, though this way seems fine too, I think it just sticks out a bit because
the rest of the numbery parameters are primitives.
also nit: should annotate this parameter with `@Nullable`
i missed the first PR, but any reason this is a double instead of integer
similar to other percent based config
`decommissioningMaxPercentOfMaxSegmentsToMove`? I guess there can be millions
of segments so the numbers being dealt with here are a bit different (at least
hopefully no one is trying to actually move on the scale of that many segments
per coordinator run..) so maybe those decimal points really do make a
difference, but otoh I can't really imagine operators configuring this much
more granular than integer numbers, which I think was the reason we used
integer for the other one iirc.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
