Use prevEndRow from Extent in ConditionalTabletMutator

Fixes apache#3356

core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java

@@ -448,11 +448,6 @@ interface ConditionalTabletMutator extends TabletUpdates<ConditionalTabletMutato
      */
     ConditionalTabletMutator requireLocation(Location location);
 
-    /**
-     * Require that a tablet has the specified previous end row.
-     */
-    ConditionalTabletMutator requirePrevEndRow(Text per);
-
     /**
      * Requires the tablet to have the specified hosting goal before any changes are made.
      */

server/base/src/main/java/org/apache/accumulo/server/manager/state/AbstractTabletStateStore.java

@@ -51,7 +51,6 @@ public void setLocations(Collection<Assignment> assignments) throws DistributedS
       for (Assignment assignment : assignments) {
         var conditionalMutator = tabletsMutator.mutateTablet(assignment.tablet)
             .requireLocation(TabletMetadata.Location.future(assignment.server))
-            .requirePrevEndRow(assignment.tablet.prevEndRow())
             .putLocation(TabletMetadata.Location.current(assignment.server))
             .deleteLocation(TabletMetadata.Location.future(assignment.server)).deleteSuspension();
 
@@ -81,8 +80,8 @@ public void setFutureLocations(Collection<Assignment> assignments)
     try (var tabletsMutator = ample.conditionallyMutateTablets()) {
       for (Assignment assignment : assignments) {
         tabletsMutator.mutateTablet(assignment.tablet).requireAbsentOperation()
-            .requireAbsentLocation().requirePrevEndRow(assignment.tablet.prevEndRow())
-            .deleteSuspension().putLocation(TabletMetadata.Location.future(assignment.server))
+            .requireAbsentLocation().deleteSuspension()
+            .putLocation(TabletMetadata.Location.future(assignment.server))
             .submit(tabletMetadata -> {
               Preconditions.checkArgument(tabletMetadata.getExtent().equals(assignment.tablet));
               return tabletMetadata.getLocation() != null && tabletMetadata.getLocation()
@@ -127,8 +126,8 @@ private void unassign(Collection<TabletMetadata> tablets,
           continue;
         }
 
-        var tabletMutator = tabletsMutator.mutateTablet(tm.getExtent())
-            .requireLocation(tm.getLocation()).requirePrevEndRow(tm.getExtent().prevEndRow());
+        var tabletMutator =
+            tabletsMutator.mutateTablet(tm.getExtent()).requireLocation(tm.getLocation());
 
         if (tm.hasCurrent()) {
 

server/base/src/main/java/org/apache/accumulo/server/manager/state/MetaDataStateStore.java

@@ -71,8 +71,7 @@ public void unsuspend(Collection<TabletMetadata> tablets) throws DistributedStor
         }
 
         // ELASTICITY_TODO pending #3314, add conditional mutation check that tls.suspend exists
-        tabletsMutator.mutateTablet(tm.getExtent()).requireAbsentOperation()
-            .requirePrevEndRow(tm.getExtent().prevEndRow()).deleteSuspension()
+        tabletsMutator.mutateTablet(tm.getExtent()).requireAbsentOperation().deleteSuspension()
             .submit(tabletMetadata -> tabletMetadata.getSuspend() == null);
       }
 

server/base/src/main/java/org/apache/accumulo/server/metadata/ConditionalTabletMutatorImpl.java

@@ -56,7 +56,6 @@
 import org.apache.accumulo.server.metadata.iterators.PresentIterator;
 import org.apache.accumulo.server.metadata.iterators.SetEqualityIterator;
 import org.apache.accumulo.server.metadata.iterators.TabletExistsIterator;
-import org.apache.hadoop.io.Text;
 
 import com.google.common.base.Preconditions;
 
@@ -74,6 +73,7 @@ public class ConditionalTabletMutatorImpl extends TabletMutatorBase<Ample.Condit
   private final KeyExtent extent;
 
   private boolean sawOperationRequirement = false;
+  private boolean checkPrevEndRow = true;
 
   protected ConditionalTabletMutatorImpl(Ample.ConditionalTabletsMutator parent,
       ServerContext context, KeyExtent extent, Consumer<ConditionalMutation> mutationConsumer,
@@ -107,16 +107,6 @@ public Ample.ConditionalTabletMutator requireLocation(Location location) {
     return this;
   }
 
-  @Override
-  public Ample.ConditionalTabletMutator requirePrevEndRow(Text per) {
-    Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate.");
-    Condition c =
-        new Condition(PREV_ROW_COLUMN.getColumnFamily(), PREV_ROW_COLUMN.getColumnQualifier())
-            .setValue(encodePrevEndRow(per).get());
-    mutation.addCondition(c);
-    return this;
-  }
-
   @Override
   public Ample.ConditionalTabletMutator requireHostingGoal(TabletHostingGoal goal) {
     Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate.");
@@ -143,6 +133,7 @@ public Ample.ConditionalTabletMutator requireAbsentTablet() {
     Condition c = new Condition("", "").setIterators(is);
     mutation.addCondition(c);
     sawOperationRequirement = true;
+    checkPrevEndRow = false;
     return this;
   }
 
@@ -168,8 +159,7 @@ public Ample.ConditionalTabletMutator requireOperation(TabletOperationId opid) {
   private void requireSameSingle(TabletMetadata tabletMetadata, ColumnType type) {
     switch (type) {
       case PREV_ROW:
-        requirePrevEndRow(tabletMetadata.getPrevEndRow());
-        break;
+        throw new IllegalStateException("PREV_ROW already set from Extent");
       case COMPACT_ID: {
         Condition c =
             new Condition(COMPACT_COLUMN.getColumnFamily(), COMPACT_COLUMN.getColumnQualifier());
@@ -251,6 +241,12 @@ public Ample.ConditionalTabletMutator requireSame(TabletMetadata tabletMetadata,
   public void submit(Ample.RejectionHandler rejectionCheck) {
     Preconditions.checkState(updatesEnabled, "Cannot make updates after calling mutate.");
     Preconditions.checkState(sawOperationRequirement, "No operation requirements were seen");
+    if (checkPrevEndRow && extent.prevEndRow() != null) {
+      Condition c =
+          new Condition(PREV_ROW_COLUMN.getColumnFamily(), PREV_ROW_COLUMN.getColumnQualifier())
+              .setValue(encodePrevEndRow(extent.prevEndRow()).get());
+      mutation.addCondition(c);
+    }
     getMutation();
     mutationConsumer.accept(mutation);
     rejectionHandlerConsumer.accept(extent, rejectionCheck);

server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java

@@ -630,8 +630,7 @@ public void requestTabletHosting(TInfo tinfo, TCredentials credentials, String t
         if (recentHostingRequest.getIfPresent(ke) == null) {
           mutator.mutateTablet(ke).requireAbsentOperation()
               .requireHostingGoal(TabletHostingGoal.ONDEMAND).requireAbsentLocation()
-              .requirePrevEndRow(ke.prevEndRow()).setHostingRequested()
-              .submit(TabletMetadata::getHostingRequested);
+              .setHostingRequested().submit(TabletMetadata::getHostingRequested);
         } else {
           log.trace("Ignoring hosting request because it was recently requested {}", ke);
         }

server/manager/src/main/java/org/apache/accumulo/manager/compaction/coordinator/CompactionCoordinator.java

@@ -563,7 +563,7 @@ private ExternalCompactionMetadata reserveCompaction(CompactionJobQueues.MetaJob
         // any data that is read from the tablet to make a decision about if it can compact or not
         // must be included in the requireSame call
         var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
-            .requireSame(tabletMetadata, PREV_ROW, FILES, SELECTED, ECOMP);
+            .requireSame(tabletMetadata, FILES, SELECTED, ECOMP);
 
         var ecid = ExternalCompactionId.of(externalCompactionId);
         tabletMutator.putExternalCompaction(ecid, ecm);
@@ -926,7 +926,7 @@ private TabletMetadata commitCompaction(TCompactionStats stats, ExternalCompacti
 
       try (var tabletsMutator = ctx.getAmple().conditionallyMutateTablets()) {
         var tabletMutator = tabletsMutator.mutateTablet(extent).requireAbsentOperation()
-            .requireCompaction(ecid).requireSame(tablet, PREV_ROW, FILES, LOCATION);
+            .requireCompaction(ecid).requireSame(tablet, FILES, LOCATION);
 
         if (ecm.getKind() == CompactionKind.USER || ecm.getKind() == CompactionKind.SELECTOR) {
           tabletMutator.requireSame(tablet, SELECTED, COMPACTED);
@@ -1050,7 +1050,7 @@ void compactionFailed(Map<ExternalCompactionId,KeyExtent> compactions) {
         try {
           ctx.requireNotDeleted(extent.tableId());
           tabletsMutator.mutateTablet(extent).requireAbsentOperation().requireCompaction(ecid)
-              .requirePrevEndRow(extent.prevEndRow()).deleteExternalCompaction(ecid)
+              .deleteExternalCompaction(ecid)
               .submit(tabletMetadata -> !tabletMetadata.getExternalCompactions().containsKey(ecid));
         } catch (TableDeletedException e) {
           LOG.warn("Table {} was deleted, unable to update metadata for compaction failure.",

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/LoadFiles.java

@@ -144,9 +144,9 @@ void load(List<TabletMetadata> tablets, Files files) {
               conditionalMutator.mutateTablet(tablet.getExtent()).requireAbsentOperation();
 
           if (setTime) {
-            tabletMutator.requireSame(tablet, PREV_ROW, LOADED, TIME, LOCATION);
+            tabletMutator.requireSame(tablet, LOADED, TIME, LOCATION);
           } else {
-            tabletMutator.requireSame(tablet, PREV_ROW, LOADED);
+            tabletMutator.requireSame(tablet, LOADED);
           }
 
           filesToLoad.forEach((f, v) -> {

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CleanUp.java

@@ -68,7 +68,7 @@ public long isReady(long tid, Manager manager) throws Exception {
       for (TabletMetadata tablet : tablets) {
         if (tablet.getCompacted().contains(tid)) {
           tabletsMutator.mutateTablet(tablet.getExtent()).requireAbsentOperation()
-              .requireSame(tablet, PREV_ROW, COMPACTED).deleteCompacted(tid)
+              .requireSame(tablet, COMPACTED).deleteCompacted(tid)
               .submit(tabletMetadata -> !tabletMetadata.getCompacted().contains(tid));
         }
       }

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/compact/CompactionDriver.java

@@ -171,7 +171,7 @@ public int updateAndCheckTablets(Manager manager, long tid)
               FateTxId.formatTid(tid), tablet.getExtent());
           // this tablet has no files try to mark it as done
           tabletsMutator.mutateTablet(tablet.getExtent()).requireAbsentOperation()
-              .requireSame(tablet, PREV_ROW, FILES, COMPACTED).putCompacted(tid)
+              .requireSame(tablet, FILES, COMPACTED).putCompacted(tid)
               .submit(tabletMetadata -> tabletMetadata.getCompacted().contains(tid));
         } else if (tablet.getSelectedFiles() == null && tablet.getExternalCompactions().isEmpty()) {
           // there are no selected files
@@ -201,11 +201,11 @@ public int updateAndCheckTablets(Manager manager, long tid)
           if (filesToCompact.isEmpty()) {
             // no files were selected so mark the tablet as compacted
             tabletsMutator.mutateTablet(tablet.getExtent()).requireAbsentOperation()
-                .requireSame(tablet, PREV_ROW, FILES, SELECTED, ECOMP, COMPACTED).putCompacted(tid)
+                .requireSame(tablet, FILES, SELECTED, ECOMP, COMPACTED).putCompacted(tid)
                 .submit(tabletMetadata -> tabletMetadata.getCompacted().contains(tid));
           } else {
             var mutator = tabletsMutator.mutateTablet(tablet.getExtent()).requireAbsentOperation()
-                .requireSame(tablet, PREV_ROW, FILES, SELECTED, ECOMP, COMPACTED);
+                .requireSame(tablet, FILES, SELECTED, ECOMP, COMPACTED);
             var selectedFiles =
                 new SelectedFiles(filesToCompact, tablet.getFiles().equals(filesToCompact), tid);
 
@@ -309,7 +309,7 @@ private void cleanupTabletMetadata(long tid, Manager manager) throws Exception {
 
           if (needsUpdate.test(tablet)) {
             var mutator = tabletsMutator.mutateTablet(tablet.getExtent()).requireAbsentOperation()
-                .requireSame(tablet, PREV_ROW, COMPACTED, SELECTED);
+                .requireSame(tablet, COMPACTED, SELECTED);
             if (tablet.getSelectedFiles() != null
                 && tablet.getSelectedFiles().getFateTxId() == tid) {
               mutator.deleteSelectedFiles();

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/PreSplit.java

@@ -98,7 +98,7 @@ public long isReady(long tid, Manager manager) throws Exception {
       try (var tabletsMutator = manager.getContext().getAmple().conditionallyMutateTablets()) {
 
         tabletsMutator.mutateTablet(splitInfo.getOriginal()).requireAbsentOperation()
-            .requireSame(tabletMetadata, LOCATION, PREV_ROW).putOperation(opid)
+            .requireSame(tabletMetadata, LOCATION).putOperation(opid)
             .submit(tmeta -> opid.equals(tmeta.getOperationId()));
 
         Map<KeyExtent,Ample.ConditionalResult> results = tabletsMutator.process();

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/SplitInfo.java

@@ -73,8 +73,6 @@ SortedSet<KeyExtent> getTablets() {
 
     TreeSet<KeyExtent> tablets = new TreeSet<>();
 
-    double sum = 0;
-
     for (var split : getSplits()) {
       var extent = new KeyExtent(getOriginal().tableId(), split, prev);
       prev = split;

server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/UpdateTablets.java

@@ -217,7 +217,7 @@ private void updateExistingTablet(long tid, Manager manager, TabletMetadata tabl
       var newExtent = newTablets.last();
 
       var mutator = tabletsMutator.mutateTablet(splitInfo.getOriginal()).requireOperation(opid)
-          .requirePrevEndRow(splitInfo.getOriginal().prevEndRow()).requireAbsentLocation();
+          .requireAbsentLocation();
 
       mutator.putPrevEndRow(newExtent.prevEndRow());
 

test/src/main/java/org/apache/accumulo/test/functional/AmpleConditionalWriterIT.java

@@ -25,7 +25,6 @@
 import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.FILES;
 import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOADED;
 import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOCATION;
-import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.PREV_ROW;
 import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SELECTED;
 import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.TIME;
 import static org.apache.accumulo.core.util.LazySingletons.GSON;
@@ -201,7 +200,7 @@ public void testFiles() throws Exception {
 
       var tm1 = TabletMetadata.builder(e1).putFile(stf1, dfv).putFile(stf2, dfv).putFile(stf3, dfv)
           .build();
-      ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm1, PREV_ROW, FILES)
+      ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm1, FILES)
           .putFile(stf4, new DataFileValue(0, 0)).submit(tm -> false);
       var results = ctmi.process();
       assertEquals(Status.REJECTED, results.get(e1).getStatus());
@@ -337,15 +336,15 @@ public void testSelectedFiles() throws Exception {
     // simulate a compaction where the tablet location is not set
     var ctmi = new ConditionalTabletsMutatorImpl(context);
     var tm1 = TabletMetadata.builder(e1).build(FILES, SELECTED);
-    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm1, PREV_ROW, FILES)
-        .putFile(stf1, dfv).putFile(stf2, dfv).putFile(stf3, dfv).submit(tm -> false);
+    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm1, FILES).putFile(stf1, dfv)
+        .putFile(stf2, dfv).putFile(stf3, dfv).submit(tm -> false);
     var results = ctmi.process();
     assertEquals(Status.ACCEPTED, results.get(e1).getStatus());
 
     assertEquals(Set.of(stf1, stf2, stf3), context.getAmple().readTablet(e1).getFiles());
 
     ctmi = new ConditionalTabletsMutatorImpl(context);
-    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm1, PREV_ROW, FILES, SELECTED)
+    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm1, FILES, SELECTED)
         .putSelectedFiles(new SelectedFiles(Set.of(stf1, stf2, stf3), true, 2L))
         .submit(tm -> false);
     results = ctmi.process();
@@ -357,7 +356,7 @@ public void testSelectedFiles() throws Exception {
     var tm2 = TabletMetadata.builder(e1).putFile(stf1, dfv).putFile(stf2, dfv).putFile(stf3, dfv)
         .build(SELECTED);
     ctmi = new ConditionalTabletsMutatorImpl(context);
-    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm2, PREV_ROW, FILES, SELECTED)
+    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm2, FILES, SELECTED)
         .putSelectedFiles(new SelectedFiles(Set.of(stf1, stf2, stf3), true, 2L))
         .submit(tm -> false);
     results = ctmi.process();
@@ -380,7 +379,7 @@ public void testSelectedFiles() throws Exception {
       var tm3 = TabletMetadata.builder(e1).putFile(stf1, dfv).putFile(stf2, dfv).putFile(stf3, dfv)
           .putSelectedFiles(selectedFiles).build();
       ctmi = new ConditionalTabletsMutatorImpl(context);
-      ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm3, PREV_ROW, FILES, SELECTED)
+      ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm3, FILES, SELECTED)
           .deleteSelectedFiles().submit(tm -> false);
       results = ctmi.process();
       assertEquals(Status.REJECTED, results.get(e1).getStatus());
@@ -393,7 +392,7 @@ public void testSelectedFiles() throws Exception {
     var tm5 = TabletMetadata.builder(e1).putFile(stf1, dfv).putFile(stf2, dfv).putFile(stf3, dfv)
         .putSelectedFiles(new SelectedFiles(Set.of(stf1, stf2, stf3), true, 2L)).build();
     ctmi = new ConditionalTabletsMutatorImpl(context);
-    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm5, PREV_ROW, FILES, SELECTED)
+    ctmi.mutateTablet(e1).requireAbsentOperation().requireSame(tm5, FILES, SELECTED)
         .deleteSelectedFiles().submit(tm -> false);
     results = ctmi.process();
     assertEquals(Status.ACCEPTED, results.get(e1).getStatus());