keith-turner commented on code in PR #3893:
URL: https://github.com/apache/accumulo/pull/3893#discussion_r1391816276


##########
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java:
##########
@@ -128,64 +125,37 @@ public TabletFiles(String dirName, List<LogEntry> 
logEntries,
     }
   }
 
-  // ELASTICITY_TODO this method is no longer called because volume 
replacement needs to move from
-  // the tablet server to the manager. See #3625
-  /**
-   * This method does two things. First, it switches any volumes a tablet is 
using that are
-   * configured in instance.volumes.replacements. Second, if a tablet dir is 
no longer configured
-   * for use it chooses a new tablet directory.
-   */
-  public static TabletFiles updateTabletVolumes(ServerContext context, 
ServiceLock zooLock,
-      KeyExtent extent, TabletFiles tabletFiles) {
-    List<Pair<Path,Path>> replacements = context.getVolumeReplacements();
-    if (replacements.isEmpty()) {
-      return tabletFiles;
+  public static void volumeReplacementEvaluation(final List<Pair<Path,Path>> 
replacements,
+      final TabletMetadata tm, final List<LogEntry> logsToRemove, final 
List<LogEntry> logsToAdd,
+      final List<StoredTabletFile> filesToRemove,
+      final SortedMap<ReferencedTabletFile,DataFileValue> filesToAdd) {
+    if (replacements.isEmpty() || (tm.getFilesMap().isEmpty() && 
tm.getLogs().isEmpty())) {
+      return;
     }
-    log.trace("Using volume replacements: {}", replacements);
-
-    List<LogEntry> logsToRemove = new ArrayList<>();
-    List<LogEntry> logsToAdd = new ArrayList<>();
-
-    List<StoredTabletFile> filesToRemove = new ArrayList<>();
-    SortedMap<ReferencedTabletFile,DataFileValue> filesToAdd = new TreeMap<>();
-
-    TabletFiles ret = new TabletFiles();
-
-    for (LogEntry logEntry : tabletFiles.logEntries) {
+    log.debug("Using volume replacements: {}", replacements);
+    for (LogEntry logEntry : tm.getLogs()) {
+      log.debug("Evaluating walog {} for replacement.", logEntry);
       LogEntry switchedLogEntry = switchVolumes(logEntry, replacements);
       if (switchedLogEntry != null) {
         logsToRemove.add(logEntry);
         logsToAdd.add(switchedLogEntry);
-        ret.logEntries.add(switchedLogEntry);
-        log.debug("Replacing volume {} : {} -> {}", extent, logEntry.filename,
+        log.debug("Replacing volume {} : {} -> {}", tm.getExtent(), 
logEntry.filename,
             switchedLogEntry.filename);
-      } else {
-        ret.logEntries.add(logEntry);
       }
     }
 
-    for (Entry<StoredTabletFile,DataFileValue> entry : 
tabletFiles.datafiles.entrySet()) {
+    for (Entry<StoredTabletFile,DataFileValue> entry : 
tm.getFilesMap().entrySet()) {
+      log.debug("Evaluating file {} for replacement.", 
entry.getKey().getPath());

Review Comment:
   Doing this per tablet and per file would be a lot of logging.  When working 
on compacting 1 million tablets I moved some logging from debug to trace 
because the log files got really large.
   
   ```suggestion
         log.trace("Evaluating file {} for replacement.", 
entry.getKey().getPath());
   ```



##########
server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java:
##########
@@ -128,64 +125,37 @@ public TabletFiles(String dirName, List<LogEntry> 
logEntries,
     }
   }
 
-  // ELASTICITY_TODO this method is no longer called because volume 
replacement needs to move from
-  // the tablet server to the manager. See #3625
-  /**
-   * This method does two things. First, it switches any volumes a tablet is 
using that are
-   * configured in instance.volumes.replacements. Second, if a tablet dir is 
no longer configured
-   * for use it chooses a new tablet directory.
-   */
-  public static TabletFiles updateTabletVolumes(ServerContext context, 
ServiceLock zooLock,
-      KeyExtent extent, TabletFiles tabletFiles) {
-    List<Pair<Path,Path>> replacements = context.getVolumeReplacements();
-    if (replacements.isEmpty()) {
-      return tabletFiles;
+  public static void volumeReplacementEvaluation(final List<Pair<Path,Path>> 
replacements,
+      final TabletMetadata tm, final List<LogEntry> logsToRemove, final 
List<LogEntry> logsToAdd,
+      final List<StoredTabletFile> filesToRemove,
+      final SortedMap<ReferencedTabletFile,DataFileValue> filesToAdd) {
+    if (replacements.isEmpty() || (tm.getFilesMap().isEmpty() && 
tm.getLogs().isEmpty())) {
+      return;
     }
-    log.trace("Using volume replacements: {}", replacements);
-
-    List<LogEntry> logsToRemove = new ArrayList<>();
-    List<LogEntry> logsToAdd = new ArrayList<>();
-
-    List<StoredTabletFile> filesToRemove = new ArrayList<>();
-    SortedMap<ReferencedTabletFile,DataFileValue> filesToAdd = new TreeMap<>();
-
-    TabletFiles ret = new TabletFiles();
-
-    for (LogEntry logEntry : tabletFiles.logEntries) {
+    log.debug("Using volume replacements: {}", replacements);

Review Comment:
   Do you want to leave this at debug?



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

Reply via email to