keith-turner commented on code in PR #3294:
URL: https://github.com/apache/accumulo/pull/3294#discussion_r1168968046
##########
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:
@dlmarion take a look at
https://github.com/keith-turner/accumulo/commit/1255dfbaf2cfcd1c4de4409d3214e2586120eee0
where I made an attempt at this.
I was looking at this code and the end row was giving me a headache because
the inclusivity of the key does not easily map to the inclusivity of the end
row for different cases. I realized that the batch scanner has to map ranges
to tablets and [looked at what it
did](https://github.com/apache/accumulo/blob/15c9f55280f4ce28a80b800f1e05128f33c85db3/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletLocatorImpl.java#L327).
That code uses range.afterEndKey so I gave that a try. The code for the batch
scanner is slightly different (its trying to determine if it should advance to
the next tablet, where this loop is concerned with the current tablet), but I
think the range.afterEndKey function could still be used in this code.
I still don't feel 100% about the code in my commit. Range overlapping code
is tricky and needs tests for all of the edge cases. If this commit works,
maybe its ok for now w/o more test because I would like to better integrate the
code that hosts tablets ondemand w/ the cache reads of the metadata (and maybe
part of that could be leverage the batch scanner cache code that does have lots
of unit test). We may need a follow on issue for test in the case where that
work is not done.
--
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]
