[dlmarion]

Removed the logic that enforced RFiles were located in paths that contained '/tables//'. Removed getVolume(), getTableId(), and getTabletDir() from ReferencedTabletFile and the parsing of the parts into the FileParts object.

Modified the GarbageCollector such that file candidates were matched by full URI and modified the logic that removed directories as an optimization to also remove RFiles not located in the directory.

Made related changes to volume replacment, table disk usage and scan server references.

Modified existing tests and created a new ExternalTabletFileIT that has coverage for scanning, major compaction and garbage collection.

Assisted-By: OpenAI GPT-6 Luna

[keith-turner]

Still looking over this, posting comments I have so far.

[keith-turner]

Suggested change

private final Set<TableId> tableIds;

Traced how table ids are used in the code and found we may be able to remove tracking of table ids for scan server refs if we make the following change. That would simplify this class and avoid persisting data this is not really needed.

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 6b9a4ed23c..b9301d1e25 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
@@ -202,7 +202,11 @@ public class GarbageCollectionAlgorithm {
     Set<TableId> tableIdsSeen = new HashSet<>(tableIdsBefore.size());
     try (Stream<Reference> references = gce.getReferences()) {
       references.forEach(ref -> {
-        tableIdsSeen.add(ref.getTableId());
+        if (ref.isDirectory()) {
+          // Every tablet must have a directory, but not every tablet has files. Only need to look
+          // for directories to ensure a tables tablets were seen.
+          tableIdsSeen.add(ref.getTableId());
+        }
 
         if (ref.isDirectory()) {
           ServerColumnFamily.validateDirCol(ref.getMetadataPath());

The above will avoid getting the table id when its not really needed for what the code is trying to accomplish. The other place that gets the table id is here and that case should never happen for scan refs. That case is for handling a super old relative reference in the metadata table from really old versions of Accumulo, scan server refs have always been absolute. Maybe we can change ReferenceFile to throw an UnsupportedOperationException if trying to get the table id for a scan server reference as its not expected to be called for that case.

[keith-turner]

There is a big change in complexity here. The old code was O(N*log2(M)) where this new code seems to be O(N*M). Not sure if this can be avoided, but if it can it would be nice to avoid the big jump in computation.

[keith-turner]

The comment I made elsewhere about reworking the keys in the map to be able to continue leveraging the sorted order of the map may help avoid this increase in computational complexity.

[keith-turner]

Prefixing keys with FILE| and DIR| in this map prevents some of the efficient operations that used to be possible. The old code leveraged the sorted to map to efficiently find all files that started with a given directory. This was done using range operations on the keys. I think we could restore this efficient behavior if we did the following two things.

• Normalize the paths in the map (like change hdfs://foo////a//b/ to hdfs://foo/a/b/
• Adopt the convention that all directory keys end with / and no file entries end with slash. Drop the FILE and DIR prefixes. With this convention we could have a static function isDir() that looks at the last char of the string.

With those changes, hopefully we can continue to do things similar to the way the existing code does and efficiently find all files with a given dir.