kfaraz commented on code in PR #13197:
URL: https://github.com/apache/druid/pull/13197#discussion_r1232981566
##########
server/src/main/java/org/apache/druid/server/coordinator/balancer/BalancerStrategy.java:
##########
@@ -36,77 +39,65 @@
*/
public interface BalancerStrategy
{
+
/**
- * Find the best server to move a {@link DataSegment} to according the
balancing strategy.
- * @param proposalSegment segment to move
- * @param serverHolders servers to consider as move destinations
+ * Finds the best server to move a segment to according to the balancing
strategy.
+ *
+ * @param proposalSegment segment to move
+ * @param sourceServer Server the segment is currently placed on.
+ * @param destinationServers servers to consider as move destinations
* @return The server to move to, or null if no move should be made or no
server is suitable
*/
@Nullable
- ServerHolder findNewSegmentHomeBalancer(DataSegment proposalSegment,
List<ServerHolder> serverHolders);
+ ServerHolder findDestinationServerToMoveSegment(
+ DataSegment proposalSegment,
+ ServerHolder sourceServer,
+ List<ServerHolder> destinationServers
+ );
/**
- * Finds the best servers on which to place a replica of the {@code
proposalSegment}
- * according to the balancing strategy.
+ * Finds the best servers on which to place the {@code proposalSegment}.
+ * This method can be used both for placing the first copy of a segment
+ * in the tier or a replica of the segment.
*
- * @param proposalSegment segment to replicate
- * @param serverHolders servers to consider as replica holders
- * @return Iterator over the best servers (in order) on which the replica(s)
+ * @param proposalSegment segment to place on servers
+ * @param serverHolders servers to consider as segment homes
+ * @return Iterator over the best servers (in order) on which the segment
* can be placed.
*/
- Iterator<ServerHolder> findNewSegmentHomeReplicator(
+ Iterator<ServerHolder> findServersToLoadSegment(
DataSegment proposalSegment,
List<ServerHolder> serverHolders
);
/**
- * Pick the best segments to move from one of the supplied set of servers
according to the balancing strategy.
+ * Picks segments from the given set of servers based on the balancing
strategy.
+ * Default behaviour is to pick segments using reservoir sampling.
*
- * @param serverHolders set of historicals to consider for moving segments
- * @param broadcastDatasources Datasources that contain segments which were
loaded via broadcast rules.
- * Balancing strategies should avoid rebalancing
segments for such datasources, since
- * they should be loaded on all servers anyway.
- * NOTE: this should really be handled on a
per-segment basis, to properly support
- * the interval or period-based broadcast
rules. For simplicity of the initial
- * implementation, only forever broadcast
rules are supported.
- * @param reservoirSize the reservoir size maintained by the Reservoir
Sampling algorithm.
- * @return Iterator for set of {@link BalancerSegmentHolder} containing
segment to move and server they currently
- * reside on, or empty if there are no segments to pick from (i. e. all
provided serverHolders are empty).
+ * @param serverHolders Set of historicals to consider for picking
segments
+ * @param broadcastDatasources Segments belonging to these datasources will
not
+ * be picked for balancing, since they should be
+ * loaded on all servers anyway.
+ * @param maxSegmentsToPick Maximum number of segments to pick
+ * @param pickLoadingSegments If true, picks only segments currently being
+ * loaded on a server. If false, picks segments
+ * already loaded on a server.
+ * @return Iterator over {@link BalancerSegmentHolder}s, each of which
contains
+ * a segment picked for moving and the server currently serving/loading it.
*/
default Iterator<BalancerSegmentHolder> pickSegmentsToMove(
List<ServerHolder> serverHolders,
Set<String> broadcastDatasources,
- int reservoirSize
+ int maxSegmentsToPick,
+ boolean pickLoadingSegments
Review Comment:
Updated:
1. Removed these methods from `BalancerStrategy`
2. Added 2 methods to `ReservoirSegmentSampler`, `pickMovableLoadingSegment`
and `pickMovableLoadedSegments`, which are used directly by the coordinator duty
--
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]
