This is an automated email from the ASF dual-hosted git repository.
domgarguilo pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 5bba2f3cf3 Improvements to scan ref removal in DatafileManager (#5674)
5bba2f3cf3 is described below
commit 5bba2f3cf36717d0e2fabbf4abb93c3455c0619e
Author: Dom G. <[email protected]>
AuthorDate: Wed Jul 2 11:36:32 2025 -0400
Improvements to scan ref removal in DatafileManager (#5674)
* Use a new scheduled task in the tablet server to clean up all scan refs.
This removes the attempt to clean them up after each scan so we can batch this
operation
---------
Co-authored-by: Keith Turner <[email protected]>
---
.../org/apache/accumulo/tserver/TabletServer.java | 2 +-
.../accumulo/tserver/tablet/DatafileManager.java | 71 +++++++++++-----------
.../org/apache/accumulo/tserver/tablet/Tablet.java | 18 +++++-
3 files changed, 51 insertions(+), 40 deletions(-)
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index d5d411db50..0cf5c2ec47 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -314,7 +314,7 @@ public class TabletServer extends AbstractServer
context.getScheduledExecutor().scheduleWithFixedDelay(Threads.createNamedRunnable(
"ScanRefCleanupTask", () ->
getOnlineTablets().values().forEach(tablet -> {
try {
- tablet.removeOrphanedScanRefs();
+ tablet.removeBatchedScanRefs();
} catch (Exception e) {
log.error("Error cleaning up stale scan references for tablet
{}",
tablet.getExtent(), e);
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
index df88ad6225..ce8c4dcd29 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
@@ -119,43 +119,27 @@ class DatafileManager {
void returnFilesForScan(Long reservationId) {
- final Set<StoredTabletFile> filesToDelete = new HashSet<>();
-
- try {
- synchronized (tablet) {
- Set<StoredTabletFile> absFilePaths =
scanFileReservations.remove(reservationId);
+ synchronized (tablet) {
+ Set<StoredTabletFile> absFilePaths =
scanFileReservations.remove(reservationId);
- if (absFilePaths == null) {
- throw new IllegalArgumentException("Unknown scan reservation id " +
reservationId);
- }
+ if (absFilePaths == null) {
+ throw new IllegalArgumentException("Unknown scan reservation id " +
reservationId);
+ }
- boolean notify = false;
- try {
- for (StoredTabletFile path : absFilePaths) {
- long refCount = fileScanReferenceCounts.decrement(path, 1);
- if (refCount == 0) {
- if (filesToDeleteAfterScan.remove(path)) {
- filesToDelete.add(path);
- }
- notify = true;
- } else if (refCount < 0) {
- throw new IllegalStateException("Scan ref count for " + path + "
is " + refCount);
- }
- }
- } finally {
- if (notify) {
- tablet.notifyAll();
+ boolean notify = false;
+ try {
+ for (StoredTabletFile path : absFilePaths) {
+ long refCount = fileScanReferenceCounts.decrement(path, 1);
+ if (refCount == 0) {
+ notify = true;
+ } else if (refCount < 0) {
+ throw new IllegalStateException("Scan ref count for " + path + "
is " + refCount);
}
}
- }
- } finally {
- // Remove scan files even if the loop above did not fully complete
because once a
- // file is in the set filesToDelete that means it was removed from
filesToDeleteAfterScan
- // and would never be added back.
- if (!filesToDelete.isEmpty()) {
- log.debug("Removing scan refs from metadata {} {}",
tablet.getExtent(), filesToDelete);
- MetadataTableUtil.removeScanFiles(tablet.getExtent(), filesToDelete,
tablet.getContext(),
- tablet.getTabletServer().getLock());
+ } finally {
+ if (notify) {
+ tablet.notifyAll();
+ }
}
}
}
@@ -185,11 +169,10 @@ class DatafileManager {
}
/**
- * Removes any scan-in-use metadata entries that were left behind when a
scan cleanup was
- * interrupted. Intended to be called periodically to clear these orphaned
scan refs once their
- * in-memory reference count reaches zero.
+ * This method will remove any scan references that have been added to
filesToDeleteAfterScan.
+ * This is meant to be called periodically as to batch the removal of scan
references.
*/
- public void removeOrphanedScanRefs() {
+ public void removeBatchedScanRefs() {
Set<StoredTabletFile> snapshot;
synchronized (tablet) {
snapshot = new HashSet<>(filesToDeleteAfterScan);
@@ -198,6 +181,20 @@ class DatafileManager {
removeFilesAfterScan(snapshot);
}
+ /**
+ * @return true if any file is no longer in use by a scan and can be
removed, false otherwise.
+ */
+ boolean canScanRefsBeRemoved() {
+ synchronized (tablet) {
+ for (var path : filesToDeleteAfterScan) {
+ if (fileScanReferenceCounts.get(path) == 0) {
+ return true;
+ }
+ }
+ return false;
+ }
+ }
+
private TreeSet<StoredTabletFile> waitForScansToFinish(Set<StoredTabletFile>
pathsToWaitFor) {
long maxWait = 10000L;
long startTime = System.currentTimeMillis();
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
index e4d9f9787c..8e296d400d 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
@@ -2220,8 +2220,22 @@ public class Tablet extends TabletBase {
return getDatafileManager().getUpdateCount();
}
- public void removeOrphanedScanRefs() {
- getDatafileManager().removeOrphanedScanRefs();
+ public void removeBatchedScanRefs() {
+ synchronized (this) {
+ if (isClosed() || isClosing()) {
+ return;
+ }
+ // return early if there are no scan files to remove
+ if (!getDatafileManager().canScanRefsBeRemoved()) {
+ return;
+ }
+ incrementWritesInProgress();
+ }
+ try {
+ getDatafileManager().removeBatchedScanRefs();
+ } finally {
+ decrementWritesInProgress();
+ }
}
TabletMemory getTabletMemory() {
