FrankChen021 commented on code in PR #19413:
URL: https://github.com/apache/druid/pull/19413#discussion_r3227000803
##########
server/src/main/java/org/apache/druid/client/BrokerServerView.java:
##########
@@ -246,6 +251,30 @@ private void
validateSegmentWatcherConfig(BrokerSegmentWatcherConfig watcherConf
&& watcherConfig.getIgnoredTiers().isEmpty()) {
throw new ISE("If configured, 'druid.broker.segment.ignoredTiers' must
be non-empty");
}
+
+ if (watcherConfig.getWatchedDeploymentGroups() != null
+ && watcherConfig.getWatchedDeploymentGroups().isEmpty()) {
+ throw new ISE("If configured,
'druid.broker.segment.watchedDeploymentGroups' must be non-empty");
+ }
+ }
+
+ /**
+ * Returns true if the server's deploymentGroup passes the watched filter,
or if the server is
+ * a realtime server type and the strict-realtime toggle is off (the
default).
+ */
+ private boolean isDeploymentGroupAllowed(DruidServerMetadata server)
+ {
+ final boolean isRealtime =
+ server.getType() == ServerType.INDEXER_EXECUTOR || server.getType() ==
ServerType.REALTIME;
+ if (isRealtime &&
!segmentWatcherConfig.isStrictRealtimeDeploymentGroupFilter()) {
+ return true;
+ }
+
+ final Set<String> watched =
segmentWatcherConfig.getWatchedDeploymentGroups();
+ if (watched != null && !watched.contains(server.getDeploymentGroup())) {
Review Comment:
[P1] Propagate deploymentGroup through inventory DruidServer construction
This filter depends on DruidServerMetadata.getDeploymentGroup(), but the
normal discovery inventory path still builds DruidServer instances through
DruidServer constructors that do not pass DruidNode.getDeploymentGroup() into
DruidServerMetadata. In a real cluster, Historicals therefore arrive here with
a null deploymentGroup, so setting druid.broker.segment.watchedDeploymentGroups
filters out all Historicals; the same lost metadata also prevents coordinator
resources from seeing active deployment groups. Please preserve the DruidNode
deploymentGroup when constructing inventory DruidServer metadata.
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/BalanceSegments.java:
##########
@@ -60,9 +63,27 @@ public DruidCoordinatorRuntimeParams
run(DruidCoordinatorRuntimeParams params)
return params;
}
- params.getDruidCluster().getManagedHistoricals().forEach(
- (tier, servers) -> new TierSegmentBalancer(tier, servers,
maxSegmentsToMove, params).run()
- );
+ final Set<String> coordinatingVersions =
params.getCoordinatorDynamicConfig().getCoordinatingVersions();
+ params.getDruidCluster().getManagedHistoricals().forEach((tier, servers)
-> {
+ if (coordinatingVersions.isEmpty()) {
+ new TierSegmentBalancer(tier, servers, maxSegmentsToMove,
params).run();
+ } else {
+ // Partition tier servers by deployment group so segments never move
across groups.
+ // Servers with no deploymentGroup form their own partition keyed
under the empty string.
+ final Map<String, Set<ServerHolder>> serversByGroup =
partitionByDeploymentGroup(servers);
+ int remainingGroups = serversByGroup.size();
+ int remainingSegmentsToMove = maxSegmentsToMove;
+ for (final Set<ServerHolder> groupServers : serversByGroup.values()) {
+ final int groupMaxSegmentsToMove =
+ (remainingSegmentsToMove + remainingGroups - 1) /
remainingGroups;
+ if (groupMaxSegmentsToMove > 0) {
+ new TierSegmentBalancer(tier, groupServers,
groupMaxSegmentsToMove, params).run();
+ remainingSegmentsToMove -= groupMaxSegmentsToMove;
Review Comment:
[P2] Do not consume unused balance budget
The per-group loop subtracts groupMaxSegmentsToMove even when
TierSegmentBalancer moves zero segments. With maxSegmentsToMove lower than the
number of groups, a first group that is already balanced can consume the whole
budget every coordinator run, leaving later skewed groups with
groupMaxSegmentsToMove == 0 indefinitely. This still honors the numeric cap,
but can permanently prevent balancing in later deployment groups; base the
budget on actual moves or rotate/fairly allocate across runs.
--
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]
