Removed unreferenced code #5935
Conversation
| * | ||
| * @since 4.0.0 | ||
| */ | ||
| static CompactableFile create(URI uri, Range range, long estimatedSize, long estimatedEntries) { |
There was a problem hiding this comment.
Added in 4.0, but not referenced. Is this a bug?
core/src/main/java/org/apache/accumulo/core/metadata/StoredTabletFile.java
Show resolved
Hide resolved
| * | ||
| * @since 4.0.0 | ||
| */ | ||
| static CompactableFile create(URI uri, Range range, long estimatedSize, long estimatedEntries) { |
There was a problem hiding this comment.
This is public API, should probably keep and maybe add test.
There was a problem hiding this comment.
The remaining CompactableFile.create method is only used in tests and in RatioBasedCompactionPlanner.FakeFileGenerator. Are users expected to create CompactableFile implementations? If not, I'm thinking that these create methods on the interface should be removed.
There was a problem hiding this comment.
Are users expected to create CompactableFile implementations?
I have found those to be useful when writing unit test for different plugins. Your plugin will be passed those and will not need to create them when running in accumulo.
There was a problem hiding this comment.
Does "useful" (especially for unit testing) imply "should be new public API", though?
There was a problem hiding this comment.
I removed the CompactableFile.create methods and just replaced their references in 3 or 4 places with new CompactableFileImpl.
There was a problem hiding this comment.
One of the methods is existing public API that was added in 2.1.0 and should be deprecated. The other was added when added ranges to metadata file entries.
Does "useful" (especially for unit testing) imply "should be new public API", though?
Yes, these are useful in the public API for two reasons. They can be used by anyone who writes one of the compaction plugins and wishes to unit test their plugin. Mocking these objects is non trivial because they have non trivial hashCode and equals functions, so attempting to mock them in a user unit test would likely be wrong and if correct would involve a lot of duplicative effort. Plugins will likely need to do set operations on these that require hashCode and equals. The easiest option for the user would be to use CompactableFileImpl in their unit test if these do not exist.
core/src/main/java/org/apache/accumulo/core/client/TableOfflineException.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/client/rfile/RFileWriter.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/client/summary/SummarizerConfiguration.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/spi/balancer/DoNothingBalancer.java
Show resolved
Hide resolved
| public TermSource(TermSource other) { | ||
| this.iter = other.iter; | ||
| this.term = other.term; | ||
| this.notFlag = other.notFlag; | ||
| this.seekColfams = other.seekColfams; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this is public API. Maybe users have a utility for this?
There was a problem hiding this comment.
Reverted changes to this class in 700ec8c
| public static String stringTopKey(SortedKeyValueIterator<Key,Value> iter) { | ||
| if (iter.hasTop()) { | ||
| return iter.getTopKey().toString(); | ||
| } | ||
| return ""; | ||
| } | ||
|
|
There was a problem hiding this comment.
Reverted changes to this class in 700ec8c
| /** | ||
| * Candidates that are malformed. | ||
| */ | ||
| INVALID |
There was a problem hiding this comment.
I'm surprised we don't need this.
| /** | ||
| * Contains additional metadata in a reserved area not for tablets | ||
| */ | ||
| public static class ReservedSection { | ||
| private static final Section section = new Section(RESERVED_PREFIX, true, null, false); | ||
|
|
||
| public static Range getRange() { | ||
| return section.getRange(); | ||
| } | ||
|
|
||
| public static String getRowPrefix() { | ||
| return section.getRowPrefix(); | ||
| } | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
This class serves as schema documentation. We don't have a need to scan the whole reserved range, so we don't use it directly, but it should still exist to provide a mechanism in code to evaluate the schema of the metadata table.
There was a problem hiding this comment.
Reverted changes to this class in 700ec8c
| public int getVersion() { | ||
| return version; | ||
| } | ||
|
|
||
| public static boolean needsUpgrade(final String json) { | ||
| var rootData = GSON.get().fromJson(json, Data.class); | ||
| int currVersion = rootData.getVersion(); | ||
| return currVersion < VERSION; | ||
| } |
There was a problem hiding this comment.
Do we read the version from the serialized metadata elsewhere now? It seems important.
3 participants
No description provided.