dlmarion commented on code in PR #3294:
URL: https://github.com/apache/accumulo/pull/3294#discussion_r1168706714


##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletLocatorImpl.java:
##########
@@ -542,70 +556,95 @@ public TabletLocation locateTablet(ClientContext context, 
Text row, boolean skip
   }
 
   @Override
-  public long onDemandTabletsOnlined() {
-    return onDemandTabletsOnlinedCount.get();
+  public long getTabletHostingRequestCount() {
+    return tabletHostingRequestCount.get();
   }
 
-  private void bringOnDemandTabletsOnline(ClientContext context, Range range)
-      throws AccumuloException, AccumuloSecurityException {
+  @VisibleForTesting
+  public void resetTabletHostingRequestCount() {
+    tabletHostingRequestCount.set(0);
+  }
 
-    // Confirm that table is in an on-demand state. Don't throw an exception
-    // if the table is not found, calling code will already handle it.
-    try {
-      String tableName = context.getTableName(tableId);
-      if (!context.tableOperations().isOnDemand(tableName)) {
-        log.trace("bringOnDemandTabletsOnline: table {} is not in ondemand 
state", tableId);
-        return;
-      }
-    } catch (TableNotFoundException e) {
-      log.trace("bringOnDemandTabletsOnline: table not found: {}", tableId);
+  @VisibleForTesting
+  public void enableTabletHostingRequests(boolean enabled) {
+    HOSTING_ENABLED.set(enabled);
+  }
+
+  private void requestTabletHosting(ClientContext context, Range range)
+      throws AccumuloException, AccumuloSecurityException, 
TableNotFoundException {
+
+    if (!HOSTING_ENABLED.get()) {
+      return;
+    }
+
+    // System tables should always be hosted
+    if (RootTable.ID == tableId || MetadataTable.ID == tableId) {
+      return;
+    }
+
+    String tableName = context.getTableName(tableId);
+    if (!context.tableOperations().isOnline(tableName)) {
+      log.trace("requestTabletHosting: table {} is not online", tableId);
       return;
     }
 
-    final Text scanRangeStart = range.getStartKey().getRow();
-    final Text scanRangeEnd = range.getEndKey().getRow();
-    // Turn the scan range into a KeyExtent and bring online all ondemand 
tablets
+    List<TKeyExtent> extentsToBringOnline =
+        findExtentsForRange(context, tableId, range, 
Set.of(TabletHostingGoal.NEVER), true);
+    if (extentsToBringOnline.isEmpty()) {
+      return;
+    }
+    log.debug("Requesting tablets be hosted: {}", extentsToBringOnline);
+    ThriftClientTypes.TABLET_MGMT.executeVoid(context,
+        client -> client.requestTabletHosting(TraceUtil.traceInfo(), 
context.rpcCreds(),
+            tableId.canonical(), extentsToBringOnline));
+    tabletHostingRequestCount.addAndGet(extentsToBringOnline.size());
+  }
+
+  public static List<TKeyExtent> findExtentsForRange(ClientContext context, 
TableId tableId,
+      Range range, Set<TabletHostingGoal> disallowedStates, boolean 
excludeHostedTablets)
+      throws AccumuloException {
+
+    final Text scanRangeStart = (range.getStartKey() == null) ? null : 
range.getStartKey().getRow();
+    final Text scanRangeEnd = (range.getEndKey() == null) ? null : 
range.getEndKey().getRow();
+    // Turn the scan range into a KeyExtent and return all tablets
     // that are overlapped by the scan range
     final KeyExtent scanRangeKE = new KeyExtent(tableId, scanRangeEnd, 
scanRangeStart);
 
-    List<TKeyExtent> extentsToBringOnline = new ArrayList<>();
+    List<TKeyExtent> extents = new ArrayList<>();
 
     TabletsMetadata m = context.getAmple().readTablets().forTable(tableId)
         .overlapping(scanRangeStart, true, null).build();
     for (TabletMetadata tm : m) {
+      if (disallowedStates.contains(tm.getHostingGoal())) {
+        throw new AccumuloException("Range: " + range + " includes tablet: " + 
tm.getExtent()
+            + " that is not in an allowable state for hosting");
+      }
       final KeyExtent tabletExtent = tm.getExtent();
       log.trace("Evaluating tablet {} against range {}", tabletExtent, 
scanRangeKE);
-      if (tm.getEndRow() != null && tm.getEndRow().compareTo(scanRangeStart) < 
0) {
+      if (scanRangeStart != null && tm.getEndRow() != null
+          && tm.getEndRow().compareTo(scanRangeStart) < 0) {
         // the end row of this tablet is before the start row, skip it
         log.trace("tablet {} is before scan start range: {}", tabletExtent, 
scanRangeStart);
         continue;
       }
-      if (tm.getPrevEndRow() != null && 
tm.getPrevEndRow().compareTo(scanRangeEnd) > 0) {
+      if (scanRangeEnd != null && tm.getPrevEndRow() != null
+          && tm.getPrevEndRow().compareTo(scanRangeEnd) > 0) {
         // the start row of this tablet is after the scan range end row, skip 
it
         log.trace("tablet {} is after scan end range: {}", tabletExtent, 
scanRangeEnd);
         continue;

Review Comment:
   @keith-turner  - I attempted to fix the logic in cd223b6 to account for the 
exclusive end row. Please take a look when you get a chance.



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