Repository navigation
Conversation
Removed the logic that enforced RFiles were located in paths that contained '/tables/<tableId>/<tabletDir>'. 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
left a comment
keith-turner
left a comment
There was a problem hiding this comment.
Still looking over this, posting comments I have so far.
| private final Value NULL_VALUE = new Value(new byte[0]); | ||
| private final Text serverAddress; | ||
| private final Text uuid; | ||
| private final Set<TableId> tableIds; |
There was a problem hiding this comment.
| 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.
|
|
||
| while (tailIter.hasNext()) { | ||
| if (tailIter.next().startsWith(blipPath)) { | ||
| while (candidateIter.hasNext()) { |
There was a problem hiding this comment.
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.
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