keith-turner commented on code in PR #2792:
URL: https://github.com/apache/accumulo/pull/2792#discussion_r970593464
##########
server/gc/src/main/java/org/apache/accumulo/gc/GCRun.java:
##########
@@ -462,4 +479,28 @@ public long getErrorsStat() {
public long getCandidatesStat() {
return candidates;
}
+
+ @Override
+ public boolean isRootTable() {
+ return level == DataLevel.ROOT;
+ }
+
+ @Override
+ public boolean isMetadataTable() {
+ return level == DataLevel.METADATA;
+ }
+
+ @Override
+ public Set<TableId> getCandidateTableIDs() {
+ if (isRootTable()) {
+ return Collections.singleton(MetadataTable.ID);
+ } else if (isMetadataTable()) {
Review Comment:
I looked how the references are obtained and I think the suggestion is
correct. In GcRun when the DataLevel is USER it will read references from the
metadata table which contains user table ids. When the DataLevel is METADATA
it will read references from the root table which will contain metadata table
id. When the DataLevel is ROOT it will read the single tablet metadata entry
for the root table from ZK (this is new after 1.x) which will contain root
table id.
In 1.1 things operated a bit differently, there were two metadata tables to
read from. In 1.1 we would look at the table being read from an infer what
kinds of table ids it contained. Now there are two metadata tables and a
single tablet metadata entry in ZK. DataLevel is used to specify to ample
which type of metadata to read for these three data levels.
Unrelated to this PR, looking at the GCRun ref code I would like to clean it
up to remove the conditional on DataLevel.Root and just pass DataLevel to
ample. I think that is an indication of the a problem in the internal ample
API.
I made suggestion to update the javadoc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
