This is an automated email from the ASF dual-hosted git repository.
ddanielr 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 1ccc3ef056 Remove Experimental property feature flag (#6416)
1ccc3ef056 is described below
commit 1ccc3ef0564916fb63bf1dac3d668f93bf553d65
Author: Daniel Roberts <[email protected]>
AuthorDate: Wed Jun 10 14:26:08 2026 -0400
Remove Experimental property feature flag (#6416)
* Remove Experimental property feature flag
Removes the `gc.remove.in.use.candidates` feature flag property that was
restricting the gc behavior from removing gc candidates for files that
were still referenced by other tablets.
* Replace hard-coded paths with method calls
Replaced the hardcoded path values with method calls for tests that used
the exact same path for file reference creation.
---
.../org/apache/accumulo/core/conf/Property.java | 5 -
.../main/java/org/apache/accumulo/gc/GCRun.java | 10 --
.../accumulo/gc/GarbageCollectionAlgorithm.java | 4 +-
.../accumulo/gc/GarbageCollectionEnvironment.java | 8 --
.../apache/accumulo/gc/GarbageCollectionTest.java | 144 ++++++++++-----------
5 files changed, 71 insertions(+), 100 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index 3d04375d84..6a4caafbda 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -996,11 +996,6 @@ public enum Property {
"The number of threads used to delete write-ahead logs and recovery
files.", "2.1.4"),
GC_DELETE_THREADS("gc.threads.delete", "16", PropertyType.COUNT,
"The number of threads used to delete RFiles.", "1.3.5"),
- @Experimental
- GC_REMOVE_IN_USE_CANDIDATES("gc.remove.in.use.candidates", "false",
PropertyType.BOOLEAN,
- "GC will remove deletion candidates that are in-use from the metadata
location. "
- + "This is expected to increase the speed of subsequent GC runs.",
- "2.1.3"),
@Deprecated(since = "2.1.1", forRemoval = true)
GC_TRASH_IGNORE("gc.trash.ignore", "false", PropertyType.BOOLEAN,
"Do not use the Trash, even if it is configured.", "1.5.0"),
diff --git a/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java
b/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java
index f56d2a21a9..9c8b1da8b5 100644
--- a/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java
+++ b/server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java
@@ -529,16 +529,6 @@ public class GCRun implements GarbageCollectionEnvironment
{
return context.getConfiguration().getBoolean(Property.GC_SAFEMODE);
}
- /**
- * Checks if InUse Candidates can be removed.
- *
- * @return value of {@link Property#GC_REMOVE_IN_USE_CANDIDATES}
- */
- @Override
- public boolean canRemoveInUseCandidates() {
- return
context.getConfiguration().getBoolean(Property.GC_REMOVE_IN_USE_CANDIDATES);
- }
-
/**
* Moves a file to trash. If this garbage collector is not using trash, this
method returns false
* and leaves the file alone. If the file is missing, this method returns
false as opposed to
diff --git
a/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
b/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
index 0e5430fa23..50b4a9f1c3 100644
---
a/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
+++
b/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
@@ -198,9 +198,7 @@ public class GarbageCollectionAlgorithm {
Set<TableId> tableIdsAfter = gce.getCandidateTableIDs();
ensureAllTablesChecked(Collections.unmodifiableSet(tableIdsBefore),
Collections.unmodifiableSet(tableIdsSeen),
Collections.unmodifiableSet(tableIdsAfter));
- if (gce.canRemoveInUseCandidates()) {
- gce.deleteGcCandidates(candidateEntriesToBeDeleted,
GcCandidateType.INUSE);
- }
+ gce.deleteGcCandidates(candidateEntriesToBeDeleted, GcCandidateType.INUSE);
}
private long removeBlipCandidates(GarbageCollectionEnvironment gce,
diff --git
a/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionEnvironment.java
b/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionEnvironment.java
index 96aae86709..086a575fb0 100644
---
a/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionEnvironment.java
+++
b/server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionEnvironment.java
@@ -29,7 +29,6 @@ import java.util.SortedMap;
import java.util.stream.Stream;
import org.apache.accumulo.core.client.TableNotFoundException;
-import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.gc.GcCandidate;
import org.apache.accumulo.core.gc.Reference;
@@ -51,13 +50,6 @@ public interface GarbageCollectionEnvironment {
*/
Iterator<GcCandidate> getCandidates() throws TableNotFoundException;
- /**
- * Used for determining if deletion of InUse candidates is enabled.
- *
- * @return value of {@link Property#GC_REMOVE_IN_USE_CANDIDATES}
- */
- boolean canRemoveInUseCandidates();
-
/**
* Given an iterator to a deletion candidate list, return a sub-list of
candidates which fit
* within provided memory constraints.
diff --git
a/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
b/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
index 74b6318029..a251215223 100644
--- a/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
+++ b/server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java
@@ -62,7 +62,6 @@ public class GarbageCollectionTest {
ArrayList<GcCandidate> fileDeletions = new ArrayList<>();
ArrayList<TableId> tablesDirsToDelete = new ArrayList<>();
TreeMap<String,Status> filesToReplicate = new TreeMap<>();
- boolean deleteInUseRefs = false;
private long timestamp = 0L;
@@ -92,11 +91,6 @@ public class GarbageCollectionTest {
return List.copyOf(candidates).iterator();
}
- @Override
- public boolean canRemoveInUseCandidates() {
- return deleteInUseRefs;
- }
-
@Override
public List<GcCandidate>
readCandidatesThatFitInMemory(Iterator<GcCandidate> candidatesIter) {
List<GcCandidate> candidatesBatch = new ArrayList<>();
@@ -239,7 +233,7 @@ public class GarbageCollectionTest {
private void assertNoCandidatesRemoved(TestGCE gce) {
assertEquals(0, gce.deletedCandidates.size(),
- "Deleted Candidates not empty: " + gce.deleteInUseRefs);
+ "Deleted Candidates not empty: " + gce.deletedCandidates.entrySet());
}
private void assertCandidateRemoved(TestGCE gce, GcCandidateType
gcCandidateType,
@@ -248,7 +242,7 @@ public class GarbageCollectionTest {
assertEquals(gcCandidateType, gce.deletedCandidates.remove(gcCandidate));
}
assertEquals(0, gce.deletedCandidates.size(),
- "Deleted Candidates not empty: " + gce.deleteInUseRefs);
+ "Deleted Candidates not empty: " + gce.deletedCandidates.entrySet());
}
// This test was created to help track down a
ConcurrentModificationException error that was
@@ -258,19 +252,20 @@ public class GarbageCollectionTest {
public void minimalDelete() throws Exception {
TestGCE gce = new TestGCE();
- gce.addCandidate("hdfs://foo:6000/accumulo/tables/4/t0/F000.rf");
- gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
- var candidate =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
- gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/6/t0/F006.rf");
+ var candOne =
gce.addCandidate("hdfs://foo:6000/accumulo/tables/4/t0/F000.rf");
+ var candTwo =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
+ var candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
+ var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/6/t0/F006.rf");
- gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
- gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
- gce.addFileReference("6", null,
"hdfs://foo.com:6000/accumulo/tables/6/t0/F006.rf");
+ gce.addFileReference("4", null, candOne.getPath());
+ gce.addFileReference("4", null, candTwo.getPath());
+ gce.addFileReference("6", null, candFour.getPath());
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
- assertFileDeleted(gce, candidate);
+ assertFileDeleted(gce, candThree);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candOne, candTwo,
candFour);
}
@Test
@@ -279,40 +274,40 @@ public class GarbageCollectionTest {
var candOne =
gce.addCandidate("hdfs://foo:6000/accumulo/tables/4/t0/F000.rf");
var candTwo =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
- gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
+ var candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
- gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
- gce.addFileReference("4", null,
"hdfs://foo:6000/accumulo/tables/4/t0/F001.rf");
+ gce.addFileReference("4", null, candOne.getPath());
+ gce.addFileReference("4", null, candTwo.getPath());
gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0//F002.rf");
- gce.addFileReference("5", null,
"hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
+ gce.addFileReference("5", null, candThree.getPath());
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
- gca.collect(gce);
- assertFileDeleted(gce);
-
- // Remove the reference to this flush file, run the GC which should not
trim it from the
- // candidates, and assert that it's gone
- gce.removeFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
+ // Remove the reference to a file in the candidates should cause it to be
removed
+ gce.removeFileReference("4", null, candOne.getPath());
gca.collect(gce);
assertFileDeleted(gce, candOne);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candTwo, candThree);
- // Removing a reference to a file that wasn't in the candidates should do
nothing
- gce.removeFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F002.rf");
- gca.collect(gce);
- assertFileDeleted(gce);
-
+ candTwo =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
// Remove the reference to a file in the candidates should cause it to be
removed
- gce.removeFileReference("4", null,
"hdfs://foo:6000/accumulo/tables/4/t0/F001.rf");
+ gce.removeFileReference("4", null, candTwo.getPath());
gca.collect(gce);
assertFileDeleted(gce, candTwo);
+ assertNoCandidatesRemoved(gce);
// Adding more candidates which do not have references should be removed
- var candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F003.rf");
- var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F004.rf");
+ var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F003.rf");
+ var candFive =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F004.rf");
gca.collect(gce);
- assertFileDeleted(gce, candThree, candFour);
+ assertFileDeleted(gce, candFour, candFive);
+ assertNoCandidatesRemoved(gce);
+ // Removing a reference to a file that wasn't in the candidates should do
nothing
+ gce.removeFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F002.rf");
+ gca.collect(gce);
+ assertFileDeleted(gce);
+ assertNoCandidatesRemoved(gce);
}
/*
@@ -328,7 +323,7 @@ public class GarbageCollectionTest {
var candOne =
gce.addCandidate("hdfs://foo:6000/accumulo/tables/4/t0/F000.rf");
var candTwo =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
- gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
+ var candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
int counter = 0;
// items to be removed from candidates
@@ -358,37 +353,44 @@ public class GarbageCollectionTest {
counter++;
}
- gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
- gce.addFileReference("4", null,
"hdfs://foo:6000/accumulo/tables/4/t0/F001.rf");
+ gce.addFileReference("4", null, candOne.getPath());
+ gce.addFileReference("4", null, candTwo.getPath());
gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0//F002.rf");
- gce.addFileReference("5", null,
"hdfs://foo.com:6000/accumulo/tables/5/t0/F005.rf");
+ gce.addFileReference("5", null, candThree.getPath());
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
assertFileDeleted(gce, toBeRemoved);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candOne, candTwo,
candThree);
- // Remove the reference to this flush file, run the GC which should not
trim it from the
- // candidates, and assert that it's gone
- gce.removeFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
+ // Add the candidate back in for the file removal
+ candOne = gce.addCandidate("hdfs://foo:6000/accumulo/tables/4/t0/F000.rf");
+ // Remove the reference to a file in the candidates should cause it to be
removed
+ gce.removeFileReference("4", null, candOne.getPath());
gca.collect(gce);
assertFileDeleted(gce, candOne);
+ assertNoCandidatesRemoved(gce);
// Removing a reference to a file that wasn't in the candidates should do
nothing
gce.removeFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F002.rf");
gca.collect(gce);
assertFileDeleted(gce);
+ assertNoCandidatesRemoved(gce);
// Remove the reference to a file in the candidates should cause it to be
removed
- gce.removeFileReference("4", null,
"hdfs://foo:6000/accumulo/tables/4/t0/F001.rf");
+ candTwo =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
+ gce.removeFileReference("4", null, candTwo.getPath());
gca.collect(gce);
assertFileDeleted(gce, candTwo);
+ assertNoCandidatesRemoved(gce);
// Adding more candidates which do no have references should be removed
- var candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F003.rf");
- var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F004.rf");
+ var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F003.rf");
+ var candFive =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F004.rf");
gca.collect(gce);
- assertFileDeleted(gce, candThree, candFour);
+ assertFileDeleted(gce, candFour, candFive);
+ assertNoCandidatesRemoved(gce);
}
/**
@@ -398,19 +400,20 @@ public class GarbageCollectionTest {
public void emptyPathsTest() throws Exception {
TestGCE gce = new TestGCE();
- gce.addCandidate("hdfs://foo:6000/accumulo/tables/4//t0//F000.rf");
- gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4//t0//F001.rf");
- var candidate =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5//t0//F005.rf");
- gce.addCandidate("hdfs://foo.com:6000/accumulo//tables//6/t0/F006.rf");
+ var candOne =
gce.addCandidate("hdfs://foo:6000/accumulo/tables/4//t0//F000.rf");
+ var candTwo =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4//t0//F001.rf");
+ var candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/5//t0//F005.rf");
+ var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo//tables//6/t0/F006.rf");
- gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4//t0//F000.rf");
- gce.addFileReference("4", null,
"hdfs://foo.com:6000/accumulo/tables/4//t0//F001.rf");
- gce.addFileReference("6", null,
"hdfs://foo.com:6000/accumulo//tables//6/t0/F006.rf");
+ gce.addFileReference("4", null, candOne.getPath());
+ gce.addFileReference("4", null, candTwo.getPath());
+ gce.addFileReference("6", null, candFour.getPath());
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
- assertFileDeleted(gce, candidate);
+ assertFileDeleted(gce, candThree);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candOne, candTwo,
candFour);
}
@Test
@@ -432,7 +435,10 @@ public class GarbageCollectionTest {
// All candidates currently have references
gca.collect(gce);
assertFileDeleted(gce);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candOne, candTwo,
candThree);
+ // Add a single reference but three possible locations for delete.
+ candOne = gce.addCandidate("/4/t0/F000.rf");
List<String[]> refsToRemove = new ArrayList<>();
refsToRemove.add(new String[] {"4", "/t0/F000.rf"});
refsToRemove.add(new String[] {"5", "../4/t0/F000.rf"});
@@ -444,27 +450,37 @@ public class GarbageCollectionTest {
gce.removeFileReference(refsToRemove.get(i)[0], null,
refsToRemove.get(i)[1]);
gca.collect(gce);
assertFileDeleted(gce);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candOne);
+ candOne = gce.addCandidate("/4/t0/F000.rf");
}
gce.removeFileReference(refsToRemove.get(2)[0], null,
refsToRemove.get(2)[1]);
gca.collect(gce);
assertFileDeleted(gce, candOne);
+ assertNoCandidatesRemoved(gce);
+ candThree =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F001.rf");
gce.removeFileReference("4", null, "/t0/F001.rf");
gca.collect(gce);
assertFileDeleted(gce, candThree);
+ assertNoCandidatesRemoved(gce);
// add absolute candidate for file that already has a relative candidate
var candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F002.rf");
gca.collect(gce);
assertFileDeleted(gce);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candFour);
+ candTwo = gce.addCandidate("/4/t0/F002.rf");
+ candFour =
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/4/t0/F002.rf");
gce.removeFileReference("4", null, "/t0/F002.rf");
gca.collect(gce);
assertFileDeleted(gce, candFour);
+ assertNoCandidatesRemoved(gce);
gca.collect(gce);
assertFileDeleted(gce, candTwo);
+ assertNoCandidatesRemoved(gce);
}
@Test
@@ -925,16 +941,9 @@ public class GarbageCollectionTest {
gce.addFileReference("6", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
- gce.deleteInUseRefs = false;
// All candidates currently have references
gca.collect(gce);
assertFileDeleted(gce);
- assertNoCandidatesRemoved(gce);
-
- // Enable InUseRefs to be removed if the file ref is found.
- gce.deleteInUseRefs = true;
- gca.collect(gce);
- assertFileDeleted(gce);
assertCandidateRemoved(gce, GcCandidateType.INUSE, candidate);
var cand1 = gce.addCandidate("/9/t0/F003.rf");
@@ -968,15 +977,6 @@ public class GarbageCollectionTest {
gce.addFileReference("+r", null,
"hdfs://foo.com:6000/accumulo/tables/4/t0/F000.rf");
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
- gce.deleteInUseRefs = false;
- // No InUse Candidates should be removed.
- gca.collect(gce);
- assertFileDeleted(gce);
- assertNoCandidatesRemoved(gce);
-
- gce.deleteInUseRefs = true;
- // Due to the gce Datalevel of ROOT, InUse candidate deletion is not
supported regardless of
- // property setting.
gca.collect(gce);
assertFileDeleted(gce);
assertNoCandidatesRemoved(gce);
@@ -1019,9 +1019,6 @@ public class GarbageCollectionTest {
assertEquals(0, gce.candidates.size());
- // Now enable InUse deletions
- gce.deleteInUseRefs = true;
-
// Add deletion candidate for a directory.
var candidate = new GcCandidate("6/t-0/", 10L);
gce.candidates.add(candidate);
@@ -1050,7 +1047,6 @@ public class GarbageCollectionTest {
gce.addFileReference("4", null, "/t0/F000.rf");
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
- gce.deleteInUseRefs = true;
gca.collect(gce);
assertFileDeleted(gce, candTwo);
