(accumulo) branch elasticity updated: conditionally removes tables scan files (#4436)

[kturner]

This is an automated email from the ASF dual-hosted git repository.

kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/elasticity by this push:
     new 5a669c9d1f conditionally removes tables scan files (#4436)
5a669c9d1f is described below

commit 5a669c9d1fb36e3989da0b2c7b3889fd9f496443
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Mon Apr 8 12:36:20 2024 -0400

    conditionally removes tables scan files (#4436)
---
 .../accumulo/server/util/MetadataTableUtil.java    |  9 ------
 .../accumulo/tserver/tablet/ScanfileManager.java   | 35 ++++++++++++++++++----
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git 
a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
 
b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
index e5db26344f..53dc257ef9 100644
--- 
a/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
+++ 
b/server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
@@ -37,7 +37,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
@@ -152,14 +151,6 @@ public class MetadataTableUtil {
     return newFiles;
   }
 
-  public static void removeScanFiles(KeyExtent extent, Set<StoredTabletFile> 
scanFiles,
-      ServerContext context, ServiceLock zooLock) {
-    TabletMutator tablet = context.getAmple().mutateTablet(extent);
-    scanFiles.forEach(tablet::deleteScan);
-    tablet.putZooLock(context.getZooKeeperRoot(), zooLock);
-    tablet.mutate();
-  }
-
   public static void deleteTable(TableId tableId, boolean insertDeletes, 
ServerContext context,
       ServiceLock lock) throws AccumuloException {
     try (
diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanfileManager.java
 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanfileManager.java
index e53bbf2553..5ab4780a39 100644
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanfileManager.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanfileManager.java
@@ -20,21 +20,28 @@ package org.apache.accumulo.tserver.tablet;
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.accumulo.core.dataImpl.KeyExtent;
+import org.apache.accumulo.core.lock.ServiceLock;
 import org.apache.accumulo.core.metadata.StoredTabletFile;
+import org.apache.accumulo.core.metadata.schema.Ample;
 import org.apache.accumulo.core.metadata.schema.DataFileValue;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata.Location;
 import org.apache.accumulo.core.util.MapCounter;
 import org.apache.accumulo.core.util.Pair;
+import org.apache.accumulo.server.ServerContext;
 import org.apache.accumulo.server.fs.VolumeManager;
-import org.apache.accumulo.server.util.MetadataTableUtil;
 import org.apache.hadoop.fs.Path;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+
 class ScanfileManager {
   private final Logger log = LoggerFactory.getLogger(ScanfileManager.class);
   private final Tablet tablet;
@@ -54,6 +61,23 @@ class ScanfileManager {
     }
   }
 
+  static void removeScanFiles(KeyExtent extent, Set<StoredTabletFile> 
scanFiles,
+      ServerContext context, Location currLocation, ServiceLock zooLock) {
+    try (var mutator = context.getAmple().conditionallyMutateTablets()) {
+      var tabletMutator = 
mutator.mutateTablet(extent).requireLocation(currLocation);
+
+      scanFiles.forEach(tabletMutator::deleteScan);
+      tabletMutator.putZooLock(context.getZooKeeperRoot(), zooLock);
+
+      tabletMutator
+          .submit(tabletMetadata -> Collections.disjoint(scanFiles, 
tabletMetadata.getScans()));
+
+      var result = mutator.process().get(extent);
+      Preconditions.checkState(result.getStatus() == 
Ample.ConditionalResult.Status.ACCEPTED,
+          "Failed to remove scan file entries for %s", extent);
+    }
+  }
+
   Pair<Long,Map<StoredTabletFile,DataFileValue>> reserveFilesForScan() {
     synchronized (tablet) {
 
@@ -112,8 +136,9 @@ class ScanfileManager {
         // file is in the set filesToDelete that means it was removed from 
filesToDeleteAfterScan
         // and would never be added back.
         log.debug("Removing scan refs from metadata {} {}", 
tablet.getExtent(), filesToDelete);
-        // ELASTICTIY_TODO use conditional mutation
-        MetadataTableUtil.removeScanFiles(tablet.getExtent(), filesToDelete, 
tablet.getContext(),
+
+        var currLoc = 
Location.current(tablet.getTabletServer().getTabletSession());
+        removeScanFiles(tablet.getExtent(), filesToDelete, 
tablet.getContext(), currLoc,
             tablet.getTabletServer().getLock());
       }
     }
@@ -137,9 +162,9 @@ class ScanfileManager {
     }
 
     if (!filesToDelete.isEmpty()) {
-      // ELASTICTIY_TODO use conditional mutation and require the tablet 
location
       log.debug("Removing scan refs from metadata {} {}", tablet.getExtent(), 
filesToDelete);
-      MetadataTableUtil.removeScanFiles(tablet.getExtent(), filesToDelete, 
tablet.getContext(),
+      var currLoc = 
Location.current(tablet.getTabletServer().getTabletSession());
+      removeScanFiles(tablet.getExtent(), filesToDelete, tablet.getContext(), 
currLoc,
           tablet.getTabletServer().getLock());
     }
   }