[GitHub] [accumulo] dlmarion commented on a diff in pull request #3228: Fix for #2872. Makes exporttable command volume aware.

["dlmarion (via GitHub)" <gi...@apache.org>]

dlmarion commented on code in PR #3228:
URL: https://github.com/apache/accumulo/pull/3228#discussion_r1204677998


##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/tableExport/WriteExportFiles.java:
##########
@@ -186,22 +190,47 @@ public static void exportTable(VolumeManager fs, ServerContext context, String t
       dataOut.close();
       dataOut = null;
 
-      createDistcpFile(fs, exportDir, exportMetaFilePath, uniqueFiles);
+      // make a set of unique volumes from the map
+      for (String fileString : uniqueFiles.values()) {
+        String uniqueVolume = getVolumeFromString(fileString);
+        volumeSet.add(uniqueVolume);
+      }
 
+      // for each unique volume: get every matching entry in the map and send them to
+      // createDistcpFile method
+      for (String volumeString : volumeSet) {
+        Set<String> sortedVolumeSet = new TreeSet<>();
+        for (String rFileString : uniqueFiles.values()) {
+          String currentVolume = getVolumeFromString(rFileString);
+          if (currentVolume.equals(volumeString)) {
+            sortedVolumeSet.add(rFileString);
+          }
+        }
+        createDistcpFile(fs, exportDir, exportMetaFilePath, sortedVolumeSet, volumeString);
+      }

Review Comment:
   ```suggestion
     final Map<Volume, List<String>> volumeFileMap = new HashMap<>();
     final Collection<Volume> configuredVolumes = fs.getVolumes();
   
     configuredVolumes.forEach(vol -> {
       final FileSystem dfs = vol.getFileSystem();
       uniqueFiles.values().forEach(file -> {
         Path p = dfs.resolvePath(new Path(file));
         if (vol.containsPath(p) {
           if (volumeFileMap.get(vol) == null) {
             volumeFileMap.put(vol, new ArrayList<String>());
           }
           volumeFileMap.get(vol).add(file);
         }
       });
     });
   
     //Now, call create DistcpFile for each entry in volumeFileMap
   ```
   
   I feel like this might be better, it performs Path resolution and uses a method that already exists to check if the path is contained in the volume vs using this new `getVolumeFromString` method.



-- 
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