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 e6cbdb4418 Adds Static "named" constructors and updates GC (#3805)
e6cbdb4418 is described below
commit e6cbdb4418d50e6c3c99e166a0390e483401fded
Author: Daniel Roberts <ddani...@gmail.com>
AuthorDate: Tue Oct 24 12:16:31 2023 -0400
Adds Static "named" constructors and updates GC (#3805)
* Adds static named constructors to ensure that inuse scan candidates are
not removed.
* Fix possible race condition with InUse Candidates
This change writes the gcCandidates twice when performing a major
compaction to ensure that valid candidates were not removed before the
tablet mutation had completed.
Fixes: #3802
* Refactored test method name
Renamed `assertRemoved` to `assertFileRemoved` to convey that the
candidate is now an hdfs file reference that has been deleted by the GC
---
.../org/apache/accumulo/core/gc/Reference.java | 7 +
.../accumulo/core/gc/ReferenceDirectory.java | 2 +-
.../org/apache/accumulo/core/gc/ReferenceFile.java | 17 +-
.../accumulo/server/gc/AllVolumesDirectory.java | 2 +-
.../accumulo/server/metadata/ServerAmpleImpl.java | 4 +-
.../accumulo/server/util/ManagerMetadataUtil.java | 4 +
.../accumulo/server/util/MetadataTableUtil.java | 4 +-
.../main/java/org/apache/accumulo/gc/GCRun.java | 32 ++--
.../accumulo/gc/GarbageCollectionAlgorithm.java | 16 +-
.../apache/accumulo/gc/GarbageCollectionTest.java | 178 +++++++++++++--------
.../accumulo/manager/TabletGroupWatcher.java | 2 +-
.../tableOps/bulkVer1/CleanUpBulkImport.java | 2 +-
.../tableOps/bulkVer2/CleanUpBulkImport.java | 2 +-
.../accumulo/manager/upgrade/Upgrader9to10.java | 4 +-
.../manager/upgrade/Upgrader9to10Test.java | 9 +-
.../test/functional/GarbageCollectorIT.java | 2 +-
16 files changed, 181 insertions(+), 106 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/gc/Reference.java
b/core/src/main/java/org/apache/accumulo/core/gc/Reference.java
index cdffbd7eeb..4c67bfd31d 100644
--- a/core/src/main/java/org/apache/accumulo/core/gc/Reference.java
+++ b/core/src/main/java/org/apache/accumulo/core/gc/Reference.java
@@ -31,6 +31,11 @@ public interface Reference {
*/
boolean isDirectory();
+ /**
+ * Only return true if the reference is a scan.
+ */
+ boolean isScan();
+
/**
* Get the {@link TableId} of the reference.
*/
@@ -42,6 +47,8 @@ public interface Reference {
* {@link
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily}
* A directory will be read from the "srv:dir" column family:
* {@link
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily}
+ * A scan will be read from the Tablet "scan" column family:
+ * {@link
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily}
*/
String getMetadataEntry();
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/gc/ReferenceDirectory.java
b/core/src/main/java/org/apache/accumulo/core/gc/ReferenceDirectory.java
index b9a6589d9f..5491020aa4 100644
--- a/core/src/main/java/org/apache/accumulo/core/gc/ReferenceDirectory.java
+++ b/core/src/main/java/org/apache/accumulo/core/gc/ReferenceDirectory.java
@@ -28,7 +28,7 @@ public class ReferenceDirectory extends ReferenceFile {
private final String tabletDir; // t-0003
public ReferenceDirectory(TableId tableId, String dirName) {
- super(tableId, dirName);
+ super(tableId, dirName, false);
MetadataSchema.TabletsSection.ServerColumnFamily.validateDirCol(dirName);
this.tabletDir = dirName;
}
diff --git a/core/src/main/java/org/apache/accumulo/core/gc/ReferenceFile.java
b/core/src/main/java/org/apache/accumulo/core/gc/ReferenceFile.java
index 7f796e8de9..b9eece90d5 100644
--- a/core/src/main/java/org/apache/accumulo/core/gc/ReferenceFile.java
+++ b/core/src/main/java/org/apache/accumulo/core/gc/ReferenceFile.java
@@ -29,13 +29,23 @@ import org.apache.accumulo.core.data.TableId;
public class ReferenceFile implements Reference, Comparable<ReferenceFile> {
// parts of an absolute URI, like "hdfs://1.2.3.4/accumulo/tables/2a/t-0003"
public final TableId tableId; // 2a
+ public final boolean isScan;
// the exact string that is stored in the metadata
protected final String metadataEntry;
- public ReferenceFile(TableId tableId, String metadataEntry) {
+ protected ReferenceFile(TableId tableId, String metadataEntry, boolean
isScan) {
this.tableId = Objects.requireNonNull(tableId);
this.metadataEntry = Objects.requireNonNull(metadataEntry);
+ this.isScan = isScan;
+ }
+
+ public static ReferenceFile forFile(TableId tableId, String metadataEntry) {
+ return new ReferenceFile(tableId, metadataEntry, false);
+ }
+
+ public static ReferenceFile forScan(TableId tableId, String metadataEntry) {
+ return new ReferenceFile(tableId, metadataEntry, true);
}
@Override
@@ -43,6 +53,11 @@ public class ReferenceFile implements Reference,
Comparable<ReferenceFile> {
return false;
}
+ @Override
+ public boolean isScan() {
+ return isScan;
+ }
+
@Override
public TableId getTableId() {
return tableId;
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/gc/AllVolumesDirectory.java
b/server/base/src/main/java/org/apache/accumulo/server/gc/AllVolumesDirectory.java
index 2dbc1705f3..aff8dd5d03 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/gc/AllVolumesDirectory.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/gc/AllVolumesDirectory.java
@@ -32,7 +32,7 @@ import org.apache.hadoop.fs.Path;
public class AllVolumesDirectory extends ReferenceFile {
public AllVolumesDirectory(TableId tableId, String dirName) {
- super(tableId, getDeleteTabletOnAllVolumesUri(tableId, dirName));
+ super(tableId, getDeleteTabletOnAllVolumesUri(tableId, dirName), false);
}
private static String getDeleteTabletOnAllVolumesUri(TableId tableId, String
dirName) {
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metadata/ServerAmpleImpl.java
b/server/base/src/main/java/org/apache/accumulo/server/metadata/ServerAmpleImpl.java
index 65fa86b814..223a9cf112 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/metadata/ServerAmpleImpl.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/metadata/ServerAmpleImpl.java
@@ -216,7 +216,9 @@ public class ServerAmpleImpl extends AmpleImpl implements
Ample {
if (level == DataLevel.ROOT) {
if (type == GcCandidateType.INUSE) {
- // Deletion of INUSE candidates is not supported in 2.1.x.
+ // Since there is only a single root tablet, supporting INUSE
candidate deletions would add
+ // additional code complexity without any substantial benefit.
+ // Therefore, deletion of root INUSE candidates is not supported.
return;
}
mutateRootGcCandidates(rgcc -> rgcc.remove(candidates.stream()));
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
index 6073822e23..5d25027d87 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/ManagerMetadataUtil.java
@@ -180,6 +180,8 @@ public class ManagerMetadataUtil {
TServerInstance tServerInstance, Location lastLocation, ServiceLock
zooLock,
Optional<ExternalCompactionId> ecid) {
+ // Write candidates before the mutation to ensure that a process failure
after a mutation would
+ // not affect candidate creation
context.getAmple().putGcCandidates(extent.tableId(), datafilesToDelete);
TabletMutator tablet = context.getAmple().mutateTablet(extent);
@@ -204,6 +206,8 @@ public class ManagerMetadataUtil {
tablet.putZooLock(zooLock);
tablet.mutate();
+ // Write candidates again to avoid a possible race condition when removing
InUse candidates
+ context.getAmple().putGcCandidates(extent.tableId(), datafilesToDelete);
}
/**
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 34d0381673..60b16e5573 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
@@ -332,8 +332,8 @@ public class MetadataTableUtil {
if (key.getColumnFamily().equals(DataFileColumnFamily.NAME)) {
StoredTabletFile stf = new
StoredTabletFile(key.getColumnQualifierData().toString());
- bw.addMutation(
- ample.createDeleteMutation(new ReferenceFile(tableId,
stf.getMetaUpdateDelete())));
+ bw.addMutation(ample
+ .createDeleteMutation(ReferenceFile.forFile(tableId,
stf.getMetaUpdateDelete())));
}
if (ServerColumnFamily.DIRECTORY_COLUMN.hasColumns(key)) {
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 6a1b9e5962..c03d4496ca 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
@@ -189,36 +189,40 @@ public class GCRun implements
GarbageCollectionEnvironment {
// there is a lot going on in this "one line" so see below for more info
var tabletReferences = tabletStream.flatMap(tm -> {
+ var tableId = tm.getTableId();
// verify that dir and prev row entries present for to check for
complete row scan
- log.trace("tablet metadata table id: {}, end row:{}, dir:{}, saw: {},
prev row: {}",
- tm.getTableId(), tm.getEndRow(), tm.getDirName(),
tm.sawPrevEndRow(), tm.getPrevEndRow());
+ log.trace("tablet metadata table id: {}, end row:{}, dir:{}, saw: {},
prev row: {}", tableId,
+ tm.getEndRow(), tm.getDirName(), tm.sawPrevEndRow(),
tm.getPrevEndRow());
if (tm.getDirName() == null || tm.getDirName().isEmpty() ||
!tm.sawPrevEndRow()) {
- throw new IllegalStateException("possible incomplete metadata scan for
table id: "
- + tm.getTableId() + ", end row: " + tm.getEndRow() + ", dir: " +
tm.getDirName()
- + ", saw prev row: " + tm.sawPrevEndRow());
+ throw new IllegalStateException("possible incomplete metadata scan for
table id: " + tableId
+ + ", end row: " + tm.getEndRow() + ", dir: " + tm.getDirName() +
", saw prev row: "
+ + tm.sawPrevEndRow());
}
// combine all the entries read from file and scan columns in the
metadata table
- Stream<StoredTabletFile> fileStream = tm.getFiles().stream();
+ Stream<StoredTabletFile> stfStream = tm.getFiles().stream();
+ // map the files to Reference objects
+ var fileStream = stfStream.map(f -> ReferenceFile.forFile(tableId,
f.getMetaUpdateDelete()));
+
// scans are normally empty, so only introduce a layer of indirection
when needed
final var tmScans = tm.getScans();
if (!tmScans.isEmpty()) {
- fileStream = Stream.concat(fileStream, tmScans.stream());
+ var scanStream =
+ tmScans.stream().map(s -> ReferenceFile.forScan(tableId,
s.getMetaUpdateDelete()));
+ fileStream = Stream.concat(fileStream, scanStream);
}
- // map the files to Reference objects
- var stream = fileStream.map(f -> new ReferenceFile(tm.getTableId(),
f.getMetaUpdateDelete()));
- // if dirName is populated then we have a tablet directory aka srv:dir
+ // if dirName is populated, then we have a tablet directory aka srv:dir
if (tm.getDirName() != null) {
// add the tablet directory to the stream
- var tabletDir = new ReferenceDirectory(tm.getTableId(),
tm.getDirName());
- stream = Stream.concat(stream, Stream.of(tabletDir));
+ var tabletDir = new ReferenceDirectory(tableId, tm.getDirName());
+ fileStream = Stream.concat(fileStream, Stream.of(tabletDir));
}
- return stream;
+ return fileStream;
});
var scanServerRefs = context.getAmple().getScanServerFileReferences()
- .map(sfr -> new ReferenceFile(sfr.getTableId(), sfr.getPathStr()));
+ .map(sfr -> ReferenceFile.forScan(sfr.getTableId(), sfr.getPathStr()));
return Stream.concat(tabletReferences, scanServerRefs);
}
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 cc77197a41..6800b9a84d 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
@@ -144,7 +144,7 @@ public class GarbageCollectionAlgorithm {
private void removeCandidatesInUse(GarbageCollectionEnvironment gce,
SortedMap<String,GcCandidate> candidateMap) throws InterruptedException {
- List<GcCandidate> inUseCandidates = new ArrayList<>();
+ List<GcCandidate> candidateEntriesToBeDeleted = new ArrayList<>();
Set<TableId> tableIdsBefore = gce.getCandidateTableIDs();
Set<TableId> tableIdsSeen = new HashSet<>();
Iterator<Reference> iter = gce.getReferences().iterator();
@@ -163,8 +163,7 @@ public class GarbageCollectionAlgorithm {
GcCandidate gcTemp = candidateMap.remove(dir);
if (gcTemp != null) {
log.debug("Directory Candidate was still in use by dir ref: {}",
dir);
- // Intentionally not adding dir candidates to inUseCandidates as
they are only added once.
- // If dir candidates are deleted, due to being in use, nothing will
add them again.
+ // Do not add dir candidates to candidateEntriesToBeDeleted as they
are only created once.
}
} else {
String reference = ref.getMetadataEntry();
@@ -183,15 +182,18 @@ public class GarbageCollectionAlgorithm {
GcCandidate gcTemp = candidateMap.remove(relativePath);
if (gcTemp != null) {
log.debug("File Candidate was still in use: {}", relativePath);
- inUseCandidates.add(gcTemp);
+ // Prevent deletion of candidates that are still in use by scans,
because they won't be
+ // recreated once the scan is finished.
+ if (!ref.isScan()) {
+ candidateEntriesToBeDeleted.add(gcTemp);
+ }
}
String dir = relativePath.substring(0, relativePath.lastIndexOf('/'));
GcCandidate gcT = candidateMap.remove(dir);
if (gcT != null) {
log.debug("Directory Candidate was still in use by file ref: {}",
relativePath);
- // Intentionally not adding dir candidates to inUseCandidates as
they are only added once.
- // If dir candidates are deleted, due to being in use, nothing will
add them again.
+ // Do not add dir candidates to candidateEntriesToBeDeleted as they
are only created once.
}
}
}
@@ -199,7 +201,7 @@ public class GarbageCollectionAlgorithm {
ensureAllTablesChecked(Collections.unmodifiableSet(tableIdsBefore),
Collections.unmodifiableSet(tableIdsSeen),
Collections.unmodifiableSet(tableIdsAfter));
if (gce.canRemoveInUseCandidates()) {
- gce.deleteGcCandidates(inUseCandidates, GcCandidateType.INUSE);
+ gce.deleteGcCandidates(candidateEntriesToBeDeleted,
GcCandidateType.INUSE);
}
}
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 0536bda699..8e9a2d1e07 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
@@ -60,7 +60,7 @@ public class GarbageCollectionTest {
Map<String,Reference> references = new TreeMap<>();
HashSet<TableId> tableIds = new HashSet<>();
- ArrayList<GcCandidate> deletes = new ArrayList<>();
+ ArrayList<GcCandidate> fileDeletions = new ArrayList<>();
ArrayList<TableId> tablesDirsToDelete = new ArrayList<>();
TreeMap<String,Status> filesToReplicate = new TreeMap<>();
boolean deleteInUseRefs = false;
@@ -121,6 +121,9 @@ public class GarbageCollectionTest {
public void deleteGcCandidates(Collection<GcCandidate> refCandidates,
GcCandidateType type) {
// Mimic ServerAmpleImpl behavior for root InUse Candidates
if (type.equals(GcCandidateType.INUSE) &&
this.level.equals(Ample.DataLevel.ROOT)) {
+ // Since there is only a single root tablet, supporting INUSE
candidate deletions would add
+ // additional code complexity without any substantial benefit.
+ // Therefore, deletion of root INUSE candidates is not supported.
return;
}
refCandidates.forEach(gcCandidate -> deletedCandidates.put(gcCandidate,
type));
@@ -136,7 +139,7 @@ public class GarbageCollectionTest {
@Override
public void deleteConfirmedCandidates(SortedMap<String,GcCandidate>
candidateMap) {
- deletes.addAll(candidateMap.values());
+ fileDeletions.addAll(candidateMap.values());
this.candidates.removeAll(candidateMap.values());
}
@@ -147,7 +150,7 @@ public class GarbageCollectionTest {
public void addFileReference(String tableId, String endRow, String file) {
TableId tid = TableId.of(tableId);
- references.put(tableId + ":" + endRow + ":" + file, new
ReferenceFile(tid, file));
+ references.put(tableId + ":" + endRow + ":" + file,
ReferenceFile.forFile(tid, file));
tableIds.add(tid);
}
@@ -167,6 +170,17 @@ public class GarbageCollectionTest {
removeLastTableIdRef(TableId.of(tableId));
}
+ public void addScanReference(String tableId, String endRow, String scan) {
+ TableId tid = TableId.of(tableId);
+ references.put(tableId + ":" + endRow + ":scan:" + scan,
ReferenceFile.forScan(tid, scan));
+ tableIds.add(tid);
+ }
+
+ public void removeScanReference(String tableId, String endRow, String
scan) {
+ references.remove(tableId + ":" + endRow + ":scan:" + scan);
+ removeLastTableIdRef(TableId.of(tableId));
+ }
+
/*
* this is to be called from removeDirReference or removeFileReference.
*
@@ -216,12 +230,12 @@ public class GarbageCollectionTest {
}
}
- private void assertRemoved(TestGCE gce, GcCandidate... candidates) {
+ private void assertFileDeleted(TestGCE gce, GcCandidate... candidates) {
for (GcCandidate candidate : candidates) {
- assertTrue(gce.deletes.remove(candidate));
+ assertTrue(gce.fileDeletions.remove(candidate));
}
- assertEquals(0, gce.deletes.size(), "Deletes not empty: " + gce.deletes);
+ assertEquals(0, gce.fileDeletions.size(), "Deletes not empty: " +
gce.fileDeletions);
}
private void assertNoCandidatesRemoved(TestGCE gce) {
@@ -257,7 +271,7 @@ public class GarbageCollectionTest {
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
- assertRemoved(gce, candidate);
+ assertFileDeleted(gce, candidate);
}
@Test
@@ -276,29 +290,29 @@ public class GarbageCollectionTest {
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
- assertRemoved(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");
gca.collect(gce);
- assertRemoved(gce, candOne);
+ assertFileDeleted(gce, candOne);
// 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);
- assertRemoved(gce);
+ assertFileDeleted(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");
gca.collect(gce);
- assertRemoved(gce, candTwo);
+ assertFileDeleted(gce, candTwo);
// 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");
gca.collect(gce);
- assertRemoved(gce, candThree, candFour);
+ assertFileDeleted(gce, candThree, candFour);
}
@@ -353,29 +367,29 @@ public class GarbageCollectionTest {
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
- assertRemoved(gce, toBeRemoved);
+ assertFileDeleted(gce, toBeRemoved);
// 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");
gca.collect(gce);
- assertRemoved(gce, candOne);
+ assertFileDeleted(gce, candOne);
// 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);
- assertRemoved(gce);
+ assertFileDeleted(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");
gca.collect(gce);
- assertRemoved(gce, candTwo);
+ assertFileDeleted(gce, candTwo);
// 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");
gca.collect(gce);
- assertRemoved(gce, candThree, candFour);
+ assertFileDeleted(gce, candThree, candFour);
}
/**
@@ -397,7 +411,7 @@ public class GarbageCollectionTest {
GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
gca.collect(gce);
- assertRemoved(gce, candidate);
+ assertFileDeleted(gce, candidate);
}
@Test
@@ -418,7 +432,7 @@ public class GarbageCollectionTest {
// All candidates currently have references
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
List<String[]> refsToRemove = new ArrayList<>();
refsToRemove.add(new String[] {"4", "/t0/F000.rf"});
@@ -430,28 +444,28 @@ public class GarbageCollectionTest {
for (int i = 0; i < 2; i++) {
gce.removeFileReference(refsToRemove.get(i)[0], null,
refsToRemove.get(i)[1]);
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
}
gce.removeFileReference(refsToRemove.get(2)[0], null,
refsToRemove.get(2)[1]);
gca.collect(gce);
- assertRemoved(gce, candOne);
+ assertFileDeleted(gce, candOne);
gce.removeFileReference("4", null, "/t0/F001.rf");
gca.collect(gce);
- assertRemoved(gce, candThree);
+ assertFileDeleted(gce, candThree);
// 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);
- assertRemoved(gce);
+ assertFileDeleted(gce);
gce.removeFileReference("4", null, "/t0/F002.rf");
gca.collect(gce);
- assertRemoved(gce, candFour);
+ assertFileDeleted(gce, candFour);
gca.collect(gce);
- assertRemoved(gce, candTwo);
+ assertFileDeleted(gce, candTwo);
}
@Test
@@ -472,7 +486,7 @@ public class GarbageCollectionTest {
// Nothing should be removed because all candidates exist within a blip
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
// Remove the first blip
gce.blips.remove("/4/b-0");
@@ -480,18 +494,18 @@ public class GarbageCollectionTest {
// And we should lose all files in that blip and the blip directory itself
-- relative and
// absolute
gca.collect(gce);
- assertRemoved(gce, new GcCandidate("/4/b-0", 0L), new
GcCandidate("/4/b-0/F002.rf", 1L),
+ assertFileDeleted(gce, new GcCandidate("/4/b-0", 0L), new
GcCandidate("/4/b-0/F002.rf", 1L),
new GcCandidate("hdfs://foo.com:6000/accumulo/tables/4/b-0/F001.rf",
2L));
gce.blips.remove("hdfs://foo.com:6000/accumulo/tables/5/b-0");
// Same as above, we should lose relative and absolute for a relative or
absolute blip
gca.collect(gce);
- assertRemoved(gce, new GcCandidate("/5/b-0", 3L), new
GcCandidate("/5/b-0/F002.rf", 4L),
+ assertFileDeleted(gce, new GcCandidate("/5/b-0", 3L), new
GcCandidate("/5/b-0/F002.rf", 4L),
new GcCandidate("hdfs://foo.com:6000/accumulo/tables/5/b-0/F001.rf",
5L));
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
}
@Test
@@ -528,21 +542,21 @@ public class GarbageCollectionTest {
// A directory reference does not preclude a candidate file beneath that
directory from deletion
gca.collect(gce);
- assertRemoved(gce, new GcCandidate("/4/t-0/F002.rf", 1L));
+ assertFileDeleted(gce, new GcCandidate("/4/t-0/F002.rf", 1L));
// Removing the dir reference for a table will delete all tablet
directories
gce.removeDirReference("5", null);
gca.collect(gce);
- assertRemoved(gce, new
GcCandidate("hdfs://foo.com:6000/accumulo/tables/5/t-0", 2L));
+ assertFileDeleted(gce, new
GcCandidate("hdfs://foo.com:6000/accumulo/tables/5/t-0", 2L));
gce.removeDirReference("4", null);
gca.collect(gce);
- assertRemoved(gce, new GcCandidate("/4/t-0", 0L));
+ assertFileDeleted(gce, new GcCandidate("/4/t-0", 0L));
gce.removeDirReference("6", null);
gce.removeDirReference("7", null);
gca.collect(gce);
- assertRemoved(gce, new GcCandidate("/6/t-0", 3L),
+ assertFileDeleted(gce, new GcCandidate("/6/t-0", 3L),
new GcCandidate("hdfs://foo:6000/accumulo/tables/7/t-0/", 4L));
gce.removeFileReference("8", "m", "/t-0/F00.rf");
@@ -552,13 +566,13 @@ public class GarbageCollectionTest {
gce.removeFileReference("e", "m", "../c/t-0/F00.rf");
gce.removeFileReference("f", "m", "../d/t-0/F00.rf");
gca.collect(gce);
- assertRemoved(gce, new GcCandidate("/8/t-0", 5L),
+ assertFileDeleted(gce, new GcCandidate("/8/t-0", 5L),
new GcCandidate("hdfs://foo:6000/accumulo/tables/9/t-0", 6L), new
GcCandidate("/a/t-0", 7L),
new GcCandidate("hdfs://foo:6000/accumulo/tables/b/t-0", 8L), new
GcCandidate("/c/t-0", 9L),
new GcCandidate("hdfs://foo:6000/accumulo/tables/d/t-0", 10L));
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
}
@Test
@@ -596,23 +610,23 @@ public class GarbageCollectionTest {
// A directory reference does not preclude a candidate file beneath that
directory from deletion
gca.collect(gce);
- assertRemoved(gce, candidates.get(2));
+ assertFileDeleted(gce, candidates.get(2));
// Removing the dir reference for a table will delete all tablet
directories
gce.removeDirReference("5", null);
// but we need to add a file ref
gce.addFileReference("8", "m", "/t-0/F00.rf");
gca.collect(gce);
- assertRemoved(gce, candidates.get(3));
+ assertFileDeleted(gce, candidates.get(3));
gce.removeDirReference("4", null);
gca.collect(gce);
- assertRemoved(gce, candidates.get(1));
+ assertFileDeleted(gce, candidates.get(1));
gce.removeDirReference("6", null);
gce.removeDirReference("7", null);
gca.collect(gce);
- assertRemoved(gce, candidates.get(4), candidates.get(5));
+ assertFileDeleted(gce, candidates.get(4), candidates.get(5));
gce.removeFileReference("8", "m", "/t-0/F00.rf");
gce.removeFileReference("9", "m", "/t-0/F00.rf");
@@ -621,11 +635,11 @@ public class GarbageCollectionTest {
gce.removeFileReference("e", "m", "../c/t-0/F00.rf");
gce.removeFileReference("f", "m", "../d/t-0/F00.rf");
gca.collect(gce);
- assertRemoved(gce, candidates.get(6), candidates.get(7),
candidates.get(8), candidates.get(9),
- candidates.get(10), candidates.get(11));
+ assertFileDeleted(gce, candidates.get(6), candidates.get(7),
candidates.get(8),
+ candidates.get(9), candidates.get(10), candidates.get(11));
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
}
private void badRefTest(String ref) {
@@ -696,8 +710,8 @@ public class GarbageCollectionTest {
gce.addCandidate("hdfs://foo.com:6000/user/foo/tables/a/t-0/t-1/F00.rf");
gca.collect(gce);
- System.out.println(gce.deletes);
- assertRemoved(gce);
+ System.out.println(gce.fileDeletions);
+ assertFileDeleted(gce);
}
@Test
@@ -709,17 +723,17 @@ public class GarbageCollectionTest {
gce.addCandidate("/1636/default_tablet");
gce.addDirReference("1636", null, "default_tablet");
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
gce.candidates.clear();
var tempCandidate = gce.addCandidate("/1636/default_tablet/someFile");
gca.collect(gce);
- assertRemoved(gce, tempCandidate);
+ assertFileDeleted(gce, tempCandidate);
gce.addFileReference("1636", null, "/default_tablet/someFile");
gce.addCandidate("/1636/default_tablet/someFile");
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
// have an indirect file reference
gce = new TestGCE();
@@ -728,19 +742,19 @@ public class GarbageCollectionTest {
gce.addDirReference("1636", null, "default_tablet");
gce.addCandidate("/9/default_tablet/someFile");
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
// have an indirect file reference and a directory candidate
gce.candidates.clear();
gce.addCandidate("/9/default_tablet");
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
gce.candidates.clear();
gce.addCandidate("/9/default_tablet");
gce.addCandidate("/9/default_tablet/someFile");
long blipCount = gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
assertEquals(0, blipCount);
gce = new TestGCE();
@@ -748,7 +762,7 @@ public class GarbageCollectionTest {
gce.blips.add("/1636/b-0001");
gce.addCandidate("/1636/b-0001/I0000");
blipCount = gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
assertEquals(1, blipCount);
gce = new TestGCE();
@@ -762,7 +776,7 @@ public class GarbageCollectionTest {
gce.addCandidate("/1000/b-1002/I0007");
var candidate = gce.addCandidate("/1000/t-0003/I0008");
blipCount = gca.collect(gce);
- assertRemoved(gce, candidate);
+ assertFileDeleted(gce, candidate);
assertEquals(5, blipCount);
}
@@ -810,7 +824,7 @@ public class GarbageCollectionTest {
// No refs to A000002.rf, and a closed, finished repl for A000001.rf
should not preclude
// it from being deleted
- assertEquals(2, gce.deletes.size());
+ assertEquals(2, gce.fileDeletions.size());
}
@Test
@@ -829,8 +843,8 @@ public class GarbageCollectionTest {
gca.collect(gce);
// We need to replicate that one file still, should not delete it.
- assertEquals(1, gce.deletes.size());
- assertEquals(candidate, gce.deletes.get(0));
+ assertEquals(1, gce.fileDeletions.size());
+ assertEquals(candidate, gce.fileDeletions.get(0));
}
@Test
@@ -851,8 +865,8 @@ public class GarbageCollectionTest {
gca.collect(gce);
// We need to replicate that one file still, should not delete it.
- assertEquals(1, gce.deletes.size());
- assertEquals(candidate, gce.deletes.get(0));
+ assertEquals(1, gce.fileDeletions.size());
+ assertEquals(candidate, gce.fileDeletions.get(0));
}
@Test
@@ -861,7 +875,7 @@ public class GarbageCollectionTest {
TestGCE gce = new TestGCE();
- assertEquals(0, gce.deletes.size());
+ assertEquals(0, gce.fileDeletions.size());
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/1/t-00001/A000001.rf");
gce.addCandidate("hdfs://foo.com:6000/accumulo/tables/2/t-00002/A000002.rf");
@@ -873,9 +887,9 @@ public class GarbageCollectionTest {
gca.collect(gce);
// We need to replicate that one file still, should not delete it.
- assertEquals(1, gce.deletes.size());
+ assertEquals(1, gce.fileDeletions.size());
assertEquals(new
GcCandidate("hdfs://foo.com:6000/accumulo/tables/2/t-00002/A000002.rf", 1L),
- gce.deletes.get(0));
+ gce.fileDeletions.get(0));
}
@Test
@@ -915,13 +929,13 @@ public class GarbageCollectionTest {
gce.deleteInUseRefs = false;
// All candidates currently have references
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
assertNoCandidatesRemoved(gce);
// Enable InUseRefs to be removed if the file ref is found.
gce.deleteInUseRefs = true;
gca.collect(gce);
- assertRemoved(gce);
+ assertFileDeleted(gce);
assertCandidateRemoved(gce, GcCandidateType.INUSE, candidate);
var cand1 = gce.addCandidate("/9/t0/F003.rf");
@@ -932,7 +946,7 @@ public class GarbageCollectionTest {
gca.collect(gce);
assertNoCandidatesRemoved(gce);
// File references did not exist, so candidates are processed
- assertRemoved(gce, cand1, cand2);
+ assertFileDeleted(gce, cand1, cand2);
}
@Test
@@ -958,14 +972,14 @@ public class GarbageCollectionTest {
gce.deleteInUseRefs = false;
// No InUse Candidates should be removed.
gca.collect(gce);
- assertRemoved(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);
- assertRemoved(gce);
+ assertFileDeleted(gce);
assertNoCandidatesRemoved(gce);
gce.removeFileReference("+r", null, "/t0/F000.rf");
@@ -975,7 +989,7 @@ public class GarbageCollectionTest {
// With file references deleted, the GC should now process the candidates
gca.collect(gce);
- assertRemoved(gce, toBeRemoved);
+ assertFileDeleted(gce, toBeRemoved);
assertNoCandidatesRemoved(gce);
}
@@ -993,7 +1007,7 @@ public class GarbageCollectionTest {
gce.addDirReference("6", null, "t-0");
gca.collect(gce);
- assertRemoved(gce, candTwo);
+ assertFileDeleted(gce, candTwo);
assertNoCandidatesRemoved(gce);
assertEquals(1, gce.candidates.size());
@@ -1001,7 +1015,7 @@ public class GarbageCollectionTest {
gce.removeDirReference("6", null);
gca.collect(gce);
- assertRemoved(gce, candOne);
+ assertFileDeleted(gce, candOne);
assertNoCandidatesRemoved(gce);
assertEquals(0, gce.candidates.size());
@@ -1019,12 +1033,38 @@ public class GarbageCollectionTest {
gca.collect(gce);
assertCandidateRemoved(gce, GcCandidateType.INUSE, removedCandidate);
- assertRemoved(gce);
+ assertFileDeleted(gce);
// Check and make sure the InUse directory candidates are not removed.
assertEquals(1, gce.candidates.size());
assertTrue(gce.candidates.contains(candidate));
}
+ @Test
+ public void testInUseScanReferenceCandidates() throws Exception {
+ TestGCE gce = new TestGCE();
+
+ // InUse Scan Refs should not be removed.
+ var scanCandidate = gce.addCandidate("/4/t0/F010.rf");
+ var candOne = gce.addCandidate("/4/t0/F000.rf");
+ var candTwo = gce.addCandidate("/6/t0/F123.rf");
+ gce.addScanReference("4", null, "/t0/F010.rf");
+ gce.addFileReference("4", null, "/t0/F000.rf");
+
+ GarbageCollectionAlgorithm gca = new GarbageCollectionAlgorithm();
+ gce.deleteInUseRefs = true;
+
+ gca.collect(gce);
+ assertFileDeleted(gce, candTwo);
+ assertCandidateRemoved(gce, GcCandidateType.INUSE, candOne);
+ assertEquals(Set.of(scanCandidate), gce.candidates);
+
+ gce.removeScanReference("4", null, "/t0/F010.rf");
+ gca.collect(gce);
+ assertFileDeleted(gce, scanCandidate);
+ assertNoCandidatesRemoved(gce);
+ assertEquals(0, gce.candidates.size());
+ }
+
// below are tests for potential failure conditions of the GC process. Some
of these cases were
// observed on clusters. Some were hypothesis based on observations. The
result was that
// candidate entries were not removed when they should have been and
therefore files were
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
index 482bbd2f44..b9914bef1f 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
@@ -703,7 +703,7 @@ abstract class TabletGroupWatcher extends
AccumuloDaemonThread {
Key key = entry.getKey();
if (key.compareColumnFamily(DataFileColumnFamily.NAME) == 0) {
var stf = new
StoredTabletFile(key.getColumnQualifierData().toString());
- datafilesAndDirs.add(new ReferenceFile(stf.getTableId(),
stf.getMetaUpdateDelete()));
+ datafilesAndDirs.add(ReferenceFile.forFile(stf.getTableId(),
stf.getMetaUpdateDelete()));
if (datafilesAndDirs.size() > 1000) {
ample.putGcFileAndDirCandidates(extent.tableId(),
datafilesAndDirs);
datafilesAndDirs.clear();
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer1/CleanUpBulkImport.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer1/CleanUpBulkImport.java
index 7cf276dc81..1ed199a070 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer1/CleanUpBulkImport.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer1/CleanUpBulkImport.java
@@ -62,7 +62,7 @@ public class CleanUpBulkImport extends ManagerRepo {
ample.removeBulkLoadInProgressFlag(
"/" + bulkDir.getParent().getName() + "/" + bulkDir.getName());
ample.putGcFileAndDirCandidates(tableId,
- Collections.singleton(new ReferenceFile(tableId, bulkDir.toString())));
+ Collections.singleton(ReferenceFile.forFile(tableId,
bulkDir.toString())));
log.debug("removing the metadata table markers for loaded files");
ample.removeBulkLoadEntries(tableId, tid, null, null);
log.debug("releasing HDFS reservations for " + source + " and " + error);
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/CleanUpBulkImport.java
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/CleanUpBulkImport.java
index f681055513..12bbacff61 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/CleanUpBulkImport.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/bulkVer2/CleanUpBulkImport.java
@@ -59,7 +59,7 @@ public class CleanUpBulkImport extends ManagerRepo {
ample.removeBulkLoadInProgressFlag(
"/" + bulkDir.getParent().getName() + "/" + bulkDir.getName());
ample.putGcFileAndDirCandidates(info.tableId,
- Collections.singleton(new ReferenceFile(info.tableId,
bulkDir.toString())));
+ Collections.singleton(ReferenceFile.forFile(info.tableId,
bulkDir.toString())));
if (info.tableState == TableState.ONLINE) {
Text firstSplit = info.firstSplit == null ? null : new
Text(info.firstSplit);
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
index 54c3775110..6bf9e3292b 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
@@ -601,7 +601,7 @@ public class Upgrader9to10 implements Upgrader {
var tableId = TableId.of(pathNoVolume.getParent().getName());
// except bulk directories don't get an all volume prefix
if (pathNoVolume.getName().startsWith(Constants.BULK_PREFIX)) {
- return new ReferenceFile(tableId, olddelete.toString());
+ return ReferenceFile.forFile(tableId, olddelete.toString());
} else {
return new AllVolumesDirectory(tableId, tabletDir);
}
@@ -610,7 +610,7 @@ public class Upgrader9to10 implements Upgrader {
if (pathNoVolume.depth() == 4) {
Path tabletDirPath = pathNoVolume.getParent();
var tableId = TableId.of(tabletDirPath.getParent().getName());
- return new ReferenceFile(tableId, olddelete.toString());
+ return ReferenceFile.forFile(tableId, olddelete.toString());
} else {
throw new IllegalStateException("Invalid delete marker: " + olddelete);
}
diff --git
a/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader9to10Test.java
b/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader9to10Test.java
index a1ad4415e1..290208b14a 100644
---
a/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader9to10Test.java
+++
b/server/manager/src/test/java/org/apache/accumulo/manager/upgrade/Upgrader9to10Test.java
@@ -85,13 +85,13 @@ public class Upgrader9to10Test {
resolved = Upgrader9to10.resolveRelativeDelete("/5a/" + BULK_PREFIX +
"0005", VOL_PROP);
assertEquals(new Path(VOL_PROP + "/tables/5a/" + BULK_PREFIX + "0005"),
resolved);
- ref1 = new ReferenceFile(tableId5a, VOL_PROP + "/tables/5a/" + BULK_PREFIX
+ "0005");
+ ref1 = ReferenceFile.forFile(tableId5a, VOL_PROP + "/tables/5a/" +
BULK_PREFIX + "0005");
var ref2 = Upgrader9to10.switchToAllVolumes(resolved);
compareReferences(ref1, ref2);
resolved = Upgrader9to10.resolveRelativeDelete("/5a/t-0005/F0009.rf",
VOL_PROP);
assertEquals(new Path(VOL_PROP + "/tables/5a/t-0005/F0009.rf"), resolved);
- ref1 = new ReferenceFile(tableId5a, VOL_PROP +
"/tables/5a/t-0005/F0009.rf");
+ ref1 = ReferenceFile.forFile(tableId5a, VOL_PROP +
"/tables/5a/t-0005/F0009.rf");
ref2 = Upgrader9to10.switchToAllVolumes(resolved);
compareReferences(ref1, ref2);
}
@@ -123,14 +123,15 @@ public class Upgrader9to10Test {
resolved = Upgrader9to10.resolveRelativeDelete(
"hdfs://localhost:9000/accumulo/tables/5a/" + BULK_PREFIX + "0005",
VOL_PROP);
- ref1 = new ReferenceFile(tableId5a,
+ ref1 = ReferenceFile.forFile(tableId5a,
"hdfs://localhost:9000/accumulo/tables/5a/" + BULK_PREFIX + "0005");
var ref2 = Upgrader9to10.switchToAllVolumes(resolved);
compareReferences(ref1, ref2);
resolved = Upgrader9to10.resolveRelativeDelete(
"hdfs://localhost:9000/accumulo/tables/5a/t-0005/C0009.rf", VOL_PROP);
- ref1 = new ReferenceFile(tableId5a,
"hdfs://localhost:9000/accumulo/tables/5a/t-0005/C0009.rf");
+ ref1 = ReferenceFile.forFile(tableId5a,
+ "hdfs://localhost:9000/accumulo/tables/5a/t-0005/C0009.rf");
ref2 = Upgrader9to10.switchToAllVolumes(resolved);
compareReferences(ref1, ref2);
}
diff --git
a/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
b/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
index eb1cafec3f..c35238dd41 100644
---
a/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java
@@ -452,7 +452,7 @@ public class GarbageCollectorIT extends ConfigurableMacBase
{
String longpath = "aaaaaaaaaabbbbbbbbbbccccccccccddddddddddeeeeeeeeee"
+ "ffffffffffgggggggggghhhhhhhhhhiiiiiiiiiijjjjjjjjjj";
var path = String.format("file:/%020d/%s", i, longpath);
- Mutation delFlag = ample.createDeleteMutation(new
ReferenceFile(TableId.of("1"), path));
+ Mutation delFlag =
ample.createDeleteMutation(ReferenceFile.forFile(TableId.of("1"), path));
bw.addMutation(delFlag);
}
}
