cshannon commented on code in PR #4317:
URL: https://github.com/apache/accumulo/pull/4317#discussion_r1517716071
##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java:
##########
@@ -66,22 +91,68 @@ public Repo<Manager> call(FateId fateId, Manager manager)
throws Exception {
tabletMetadata.getLogs().size());
}
- SortedSet<Text> splits = SplitUtils.findSplits(manager.getContext(),
tabletMetadata);
+ var estimatedSize =
+
tabletMetadata.getFilesMap().values().stream().mapToLong(DataFileValue::getSize).sum();
+ SortedSet<Text> splits =
+ SplitUtils.findSplits(manager.getContext(), tabletMetadata,
estimatedSize);
if (extent.endRow() != null) {
splits.remove(extent.endRow());
}
if (splits.isEmpty()) {
- log.info("Tablet {} needs to split, but no split points could be found.",
- tabletMetadata.getExtent());
- // ELASTICITY_TODO record the fact that tablet is un-splittable in
metadata table in a new
- // column. Record the config used to reach this decision and a hash of
the file. The tablet
- // mgmt iterator can inspect this column and only try to split the
tablet when something has
- // changed.
+ Consumer<ConditionalResult> resultConsumer = result -> {
+ if (result.getStatus() == Status.REJECTED) {
+ log.debug("{} unsplittable metadata update for {} was rejected ",
fateId,
+ result.getExtent());
+ }
+ };
+
+ try (var tabletsMutator =
ample.conditionallyMutateTablets(resultConsumer)) {
+ // Check if we still need to split. It's possible we don't if the
unsplittable marker
+ // has already been previously set. This could happen in some
scenarios such as
+ // a compaction that shrinks a previously unsplittable tablet below
the threshold
+ // or if the threshold has been raised higher because the tablet
management iterator
+ // will try and split any time the computed metadata changes.
+ if (stillNeedsSplit(manager.getContext(), tabletMetadata,
estimatedSize)) {
+ log.info("Tablet {} needs to split, but no split points could be
found.",
+ tabletMetadata.getExtent());
+ var unSplittableMeta = computedUnsplittable
+ .orElseGet(() -> SplitUtils.toUnSplittable(manager.getContext(),
tabletMetadata));
+
+ // With the current design we don't need to require the files to be
the same
+ // for correctness as the TabletManagementIterator will detect the
difference
+ // when computing the hash and retry a new split operation if there
is not a match.
+ // But if we already know there's a change now, it would be more
efficient to fail and
+ // retry the current fate op vs completing and having the iterator
submit a new one.
+ var mutator =
tabletsMutator.mutateTablet(extent).requireAbsentOperation()
+ .requireSame(tabletMetadata,
FILES).setUnSplittable(unSplittableMeta);
+ mutator.submit(tm -> unSplittableMeta.equals(tm.getUnSplittable()));
+ } else {
+ // We no longer need to split so we can clear the marker.
+ log.info("Tablet {} no longer needs to split, deleting unsplittable
marker.",
+ tabletMetadata.getExtent());
+ var mutator =
tabletsMutator.mutateTablet(extent).requireAbsentOperation()
+ .requireSame(tabletMetadata, FILES).deleteUnSplittable();
+ mutator.submit(tm -> tm.getUnSplittable() == null);
+ }
Review Comment:
Good catch, this avoids having try try and clear the marker if it is already
null and never set
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
