keith-turner commented on code in PR #2665:
URL: https://github.com/apache/accumulo/pull/2665#discussion_r865367569


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java:
##########
@@ -453,26 +482,40 @@ public void run() {
   private void doLookups(Map<String,Map<KeyExtent,List<Range>>> binnedRanges,
       final ResultReceiver receiver, List<Column> columns) {
 
-    if (timedoutServers.containsAll(binnedRanges.keySet())) {
-      // all servers have timed out
-      throw new TimedOutException(timedoutServers);
-    }
-
-    // when there are lots of threads and a few tablet servers
-    // it is good to break request to tablet servers up, the
-    // following code determines if this is the case
     int maxTabletsPerRequest = Integer.MAX_VALUE;
-    if (numThreads / binnedRanges.size() > 1) {
-      int totalNumberOfTablets = 0;
-      for (Entry<String,Map<KeyExtent,List<Range>>> entry : 
binnedRanges.entrySet()) {
-        totalNumberOfTablets += entry.getValue().size();
-      }
 
-      maxTabletsPerRequest = totalNumberOfTablets / numThreads;
-      if (maxTabletsPerRequest == 0) {
-        maxTabletsPerRequest = 1;
+    long busyTimeout = 0;
+    Duration scanServerDispatcherDelay = null;
+    Map<String,ScanAttemptsImpl.ScanAttemptReporter> reporters = Map.of();
+
+    if (options.getConsistencyLevel().equals(ConsistencyLevel.EVENTUAL)) {

Review Comment:
   > But in that case we don't get the advantage of the code in the else 
statement.
   
   That was an intentional decision to leave the else stmt just for the 
tserver/immediate path. Was thinking for the eventual path that all control is 
delegated to the ScanServerDispatcher plugin avoiding hardcoded decisions.  The 
ScanServerDispatcher plugin does not currently support sending multiple threads 
to a single scan server (that is what the else code is doing for tservers).  I 
had wanted to refactor the ScanServerDispatcher to support this use case, but 
did not get to it.  I think this would be worthwhile and could look into that 
after this PR is merged. 



-- 
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]

Reply via email to