dlmarion commented on a change in pull request #2422:
URL: https://github.com/apache/accumulo/pull/2422#discussion_r791123988
##########
File path:
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java
##########
@@ -497,26 +499,44 @@ private void
doLookups(Map<String,Map<KeyExtent,List<Range>>> binnedRanges,
for (final String tsLocation : locations) {
final Map<KeyExtent,List<Range>> tabletsRanges =
binnedRanges.get(tsLocation);
- if (maxTabletsPerRequest == Integer.MAX_VALUE || tabletsRanges.size() ==
1) {
- QueryTask queryTask = new QueryTask(tsLocation, tabletsRanges,
failures, receiver, columns);
- queryTasks.add(queryTask);
+ if (options.isUseScanServer()) {
+ // Ignore the tablets location and find a scan server to use
+ ScanServerLocator ssl = context.getScanServerLocator();
+ tabletsRanges.forEach((k, v) -> {
+ try {
+ String location = ssl.reserveScanServer(new TabletIdImpl(k));
Review comment:
My latest commit added more tests in ScanServerIT. I need to add a
couple more tests that involve the BatchScanner, but I'm pretty well convinced
that this approach will work. I think the ScanServer locator piece might be the
most complex piece about this change when we are done with it. From a load
balancer perspective, it would be nice to know the following for each
ScanServer:
- last N extents scanned (increased probability of cache hit?).
- local TabletServers Tablets, if any (increased probability of locality or
page cache hit?)
The extent information is likely sensitive and not something we want
client-side. If we choose to use this information for load balancing, then it
might have to go into the Manager.
--
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]
