[GitHub] [accumulo] keith-turner commented on a diff in pull request #3480: WIP - update stored file metadata to include a range

["keith-turner (via GitHub)" <gi...@apache.org>]

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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org