Jackie-Jiang commented on a change in pull request #4119: Improve partition
aware routing when a server is down.
URL: https://github.com/apache/incubator-pinot/pull/4119#discussion_r276070472
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/routing/builder/BasePartitionAwareRoutingTableBuilder.java
##########
@@ -101,40 +101,59 @@ public void init(Configuration configuration,
TableConfig tableConfig, ZkHelixPr
Map<String, List<String>> routingTable = new HashMap<>();
SegmentPrunerContext prunerContext = new
SegmentPrunerContext(request.getBrokerRequest());
- // 1. Randomly pick a replica id
- int replicaId = _random.nextInt(_numReplicas);
+ // Shuffle the replica group id pool in order to satisfy:
+ // 1. Pick a replica group in an evenly distributed fashion
+ // 2. When a server is not available, the request should be distributed
evenly among other available servers.
+ int[] replicaGroupIdPool = new int[_numReplicas];
+ for (int i = 0; i < _numReplicas; i++) {
+ replicaGroupIdPool[i] = i;
+ }
+ shuffleArray(replicaGroupIdPool);
+
for (String segmentName : segmentsToQuery) {
SegmentZKMetadata segmentZKMetadata =
_segmentToZkMetadataMapping.get(segmentName);
// 2a. Check if the segment can be pruned
boolean segmentPruned = (segmentZKMetadata != null) &&
_pruner.prune(segmentZKMetadata, prunerContext);
if (!segmentPruned) {
- // 2b. Segment cannot be pruned. Assign the segment to a server with
the replica id picked above.
+ // 2b. Segment cannot be pruned. Assign the segment to a server based
on the replica group id pool
Map<Integer, String> replicaIdToServerMap =
segmentToReplicaToServerMap.get(segmentName);
- String serverName = replicaIdToServerMap.get(replicaId);
-
- // When the server is not available with this replica id, we need to
pick another available server.
- if (serverName == null) {
- if (!replicaIdToServerMap.isEmpty()) {
- serverName = replicaIdToServerMap.values().iterator().next();
- } else {
- // No server is found for this segment
- continue;
+
+ String serverName = null;
+ for (int i = 0; i < _numReplicas; i++) {
+ serverName = replicaIdToServerMap.get(replicaGroupIdPool[i]);
+ // If a server is found, update routing table for the current segment
+ if (serverName != null) {
+ break;
}
}
- List<String> segmentsForServer = routingTable.get(serverName);
- if (segmentsForServer == null) {
- segmentsForServer = new ArrayList<>();
- routingTable.put(serverName, segmentsForServer);
+
+ if (serverName != null) {
+ List<String> segmentsForServer =
routingTable.computeIfAbsent(serverName, k -> new ArrayList<>());
+ segmentsForServer.add(segmentName);
+ } else {
+ // No server is found for this segment if the code reach here
Review comment:
This method is called when we process the external view change. Don't know
if it's okay to call it on per query per segment manner. BTW, directly log
warning/error might flooding the log. Let's figure out a better way to handle
this.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
