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


##########
server/manager/src/main/java/org/apache/accumulo/manager/recovery/RecoveryManager.java:
##########
@@ -156,72 +156,68 @@ private boolean exists(final Path path) throws 
IOException {
     }
   }
 
-  public boolean recoverLogs(KeyExtent extent, Collection<Collection<String>> 
walogs)
-      throws IOException {
+  public boolean recoverLogs(KeyExtent extent, Collection<LogEntry> walogs) 
throws IOException {
     boolean recoveryNeeded = false;
 
-    for (Collection<String> logs : walogs) {
-      for (String walog : logs) {
-
-        Path switchedWalog = VolumeUtil.switchVolume(new Path(walog), 
FileType.WAL,
-            manager.getContext().getVolumeReplacements());
-        if (switchedWalog != null) {
-          // replaces the volume used for sorting, but do not change entry in 
metadata table. When
-          // the tablet loads it will change the metadata table entry. If
-          // the tablet has the same replacement config, then it will find the 
sorted log.
-          log.info("Volume replaced {} -> {}", walog, switchedWalog);
-          walog = switchedWalog.toString();
-        }
+    for (LogEntry walog : walogs) {
+
+      LogEntry switchedWalog =
+          VolumeUtil.switchVolumes(walog, 
manager.getContext().getVolumeReplacements());
+      if (switchedWalog != null) {
+        // replaces the volume used for sorting, but do not change entry in 
metadata table. When
+        // the tablet loads it will change the metadata table entry. If
+        // the tablet has the same replacement config, then it will find the 
sorted log.
+        log.info("Volume replaced {} -> {}", walog, switchedWalog);
+        walog = switchedWalog;
+      }
+
+      String sortId = walog.getUniqueID().toString();

Review Comment:
   Nice that this code is no longer parsing here.



##########
server/manager/src/main/java/org/apache/accumulo/manager/recovery/RecoveryManager.java:
##########
@@ -156,72 +156,68 @@ private boolean exists(final Path path) throws 
IOException {
     }
   }
 
-  public boolean recoverLogs(KeyExtent extent, Collection<Collection<String>> 
walogs)
-      throws IOException {
+  public boolean recoverLogs(KeyExtent extent, Collection<LogEntry> walogs) 
throws IOException {
     boolean recoveryNeeded = false;
 
-    for (Collection<String> logs : walogs) {
-      for (String walog : logs) {
-
-        Path switchedWalog = VolumeUtil.switchVolume(new Path(walog), 
FileType.WAL,

Review Comment:
   Do you know if this removed the need for  FileType.WAL?  Nice that this 
could use types instead of the enum.



##########
core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java:
##########
@@ -106,11 +112,10 @@ public static LogEntry fromMetaWalEntry(Entry<Key,Value> 
entry) {
     Preconditions.checkArgument(LogColumnFamily.NAME.equals(fam),
         "The provided metadata entry's column family is %s instead of %s", fam,
         LogColumnFamily.NAME);
-    String qualifier = entry.getKey().getColumnQualifier().toString();
-    String[] parts = qualifier.split("/", 2);
-    Preconditions.checkArgument(parts.length == 2 && parts[0].equals("-"),
-        "Malformed write-ahead log %s", qualifier);
-    return fromPath(parts[1]);
+    Text qualifier = entry.getKey().getColumnQualifier();
+    String[] parts = qualifier.toString().split("/", 2);
+    Preconditions.checkArgument(parts.length == 2, "Malformed write-ahead log 
%s", qualifier);

Review Comment:
   Can you verify the contents of `parts[0]` here?  Not sure if that is 
possible when considering possible old code.



##########
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java:
##########
@@ -256,10 +257,6 @@ private long removeFiles(Collection<Path> values) {
     return count;
   }
 
-  private UUID path2uuid(Path path) {

Review Comment:
   Nice to remove this and use centralized code.



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