Browse Source

HADOOP-17332. S3A MarkerTool -min and -max are inverted. (#2425)

This patch
* fixes the inversion
* adds a precondition check
* if the commands are supplied inverted, swaps them with a warning.
  This is to stop breaking any tests written to cope with the existing
  behavior.

Contributed by Steve Loughran
Steve Loughran 4 years ago
parent
commit
9b4faf2b51

+ 20 - 2
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/MarkerTool.java

@@ -57,6 +57,7 @@ import org.apache.hadoop.fs.s3a.impl.DirectoryPolicyImpl;
 import org.apache.hadoop.fs.s3a.impl.StoreContext;
 import org.apache.hadoop.fs.s3a.impl.StoreContext;
 import org.apache.hadoop.fs.s3a.s3guard.S3GuardTool;
 import org.apache.hadoop.fs.s3a.s3guard.S3GuardTool;
 import org.apache.hadoop.fs.shell.CommandFormat;
 import org.apache.hadoop.fs.shell.CommandFormat;
+import org.apache.hadoop.thirdparty.com.google.common.base.Preconditions;
 import org.apache.hadoop.util.DurationInfo;
 import org.apache.hadoop.util.DurationInfo;
 import org.apache.hadoop.util.ExitUtil;
 import org.apache.hadoop.util.ExitUtil;
 
 
@@ -395,10 +396,22 @@ public final class MarkerTool extends S3GuardTool {
     } else {
     } else {
       filterPolicy = null;
       filterPolicy = null;
     }
     }
+    int minMarkerCount = scanArgs.getMinMarkerCount();
+    int maxMarkerCount = scanArgs.getMaxMarkerCount();
+    if (minMarkerCount > maxMarkerCount) {
+      // swap min and max if they are wrong.
+      // this is to ensure any test scripts written to work around
+      // HADOOP-17332 and min/max swapping continue to work.
+      println(out, "Swapping -min (%d) and -max (%d) values",
+          minMarkerCount, maxMarkerCount);
+      int m = minMarkerCount;
+      minMarkerCount = maxMarkerCount;
+      maxMarkerCount = m;
+    }
     ScanResult result = scan(target,
     ScanResult result = scan(target,
         scanArgs.isDoPurge(),
         scanArgs.isDoPurge(),
-        scanArgs.getMaxMarkerCount(),
-        scanArgs.getMinMarkerCount(),
+        minMarkerCount,
+        maxMarkerCount,
         scanArgs.getLimit(),
         scanArgs.getLimit(),
         filterPolicy);
         filterPolicy);
     return result;
     return result;
@@ -513,6 +526,11 @@ public final class MarkerTool extends S3GuardTool {
       final DirectoryPolicy filterPolicy)
       final DirectoryPolicy filterPolicy)
       throws IOException, ExitUtil.ExitException {
       throws IOException, ExitUtil.ExitException {
 
 
+    // safety check: min and max are correctly ordered at this point.
+    Preconditions.checkArgument(minMarkerCount <= maxMarkerCount,
+        "The min marker count of %d is greater than the max value of %d",
+        minMarkerCount, maxMarkerCount);
+
     ScanResult result = new ScanResult();
     ScanResult result = new ScanResult();
 
 
     // Mission Accomplished
     // Mission Accomplished

+ 19 - 2
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerTool.java

@@ -259,8 +259,25 @@ public class ITestMarkerTool extends AbstractMarkerToolTest {
         AUDIT,
         AUDIT,
         m(OPT_LIMIT), 0,
         m(OPT_LIMIT), 0,
         m(OPT_OUT), audit,
         m(OPT_OUT), audit,
-        m(OPT_MIN), expectedMarkersWithBaseDir,
-        m(OPT_MAX), expectedMarkersWithBaseDir,
+        m(OPT_MIN), expectedMarkersWithBaseDir - 1,
+        m(OPT_MAX), expectedMarkersWithBaseDir + 1,
+        createdPaths.base);
+    expectMarkersInOutput(audit, expectedMarkersWithBaseDir);
+  }
+
+  @Test
+  public void testRunAuditWithExpectedMarkersSwappedMinMax() throws Throwable {
+    describe("Run a verbose audit with the min/max ranges swapped;"
+        + " see HADOOP-17332");
+    // a run under the keeping FS will create paths
+    CreatedPaths createdPaths = createPaths(getKeepingFS(), methodPath());
+    final File audit = tempAuditFile();
+    run(MARKERS, V,
+        AUDIT,
+        m(OPT_LIMIT), 0,
+        m(OPT_OUT), audit,
+        m(OPT_MIN), expectedMarkersWithBaseDir + 1,
+        m(OPT_MAX), expectedMarkersWithBaseDir - 1,
         createdPaths.base);
         createdPaths.base);
     expectMarkersInOutput(audit, expectedMarkersWithBaseDir);
     expectMarkersInOutput(audit, expectedMarkersWithBaseDir);
   }
   }