keith-turner commented on code in PR #3480:
URL: https://github.com/apache/accumulo/pull/3480#discussion_r1239255123
##########
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeManager.java:
##########
@@ -92,7 +93,7 @@ public Path getVolume(Path path) {
}
public Path removeVolume(Path path) {
- String pathString = path.toString();
+ String pathString = Objects.requireNonNull(path).toString();
Review Comment:
Why add the requireNonNull call? Seems like calling `path.toString()` would
throw an NPE if path were null just like
`Objects.requireNonNull(path).toString()` would.
##########
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java:
##########
@@ -162,11 +163,12 @@ public static TabletFiles
updateTabletVolumes(ServerContext context, ServiceLock
}
for (Entry<StoredTabletFile,DataFileValue> entry :
tabletFiles.datafiles.entrySet()) {
- String metaPath = entry.getKey().getMetaUpdateDelete();
Review Comment:
It may be possible to add a unit test for this method that ensures when
volumes are switched and there is a range that the range makes it through.
##########
core/src/main/java/org/apache/accumulo/core/metadata/AbstractTabletFile.java:
##########
@@ -62,4 +62,17 @@ public Range getRange() {
public boolean hasRange() {
return !range.isInfiniteStartKey() || !range.isInfiniteStopKey();
}
+
+ /**
+ * Return the exact string (full Json with range) that is stored in the
metadata table.
+ */
+ public abstract String getMetadata();
Review Comment:
Why were these methods relating to what is in the metadata table added here
instead of just in StoredTabletFile?
--
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]
