From 9b4faf2b51a0148c85ec6bb0b983daf0b8be52ff Mon Sep 17 00:00:00 2001
From: Steve Loughran <stevel@cloudera.com>
Date: Mon, 23 Nov 2020 20:49:42 +0000
Subject: [PATCH] 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
---
 .../hadoop/fs/s3a/tools/MarkerTool.java       | 22 +++++++++++++++++--
 .../hadoop/fs/s3a/tools/ITestMarkerTool.java  | 21 ++++++++++++++++--
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/MarkerTool.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/MarkerTool.java
index a8cdbc22c3..848b9ae632 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/MarkerTool.java
+++ b/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.StoreContext;
 import org.apache.hadoop.fs.s3a.s3guard.S3GuardTool;
 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.ExitUtil;
 
@@ -395,10 +396,22 @@ ScanResult execute(final ScanArgs scanArgs)
     } else {
       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,
         scanArgs.isDoPurge(),
-        scanArgs.getMaxMarkerCount(),
-        scanArgs.getMinMarkerCount(),
+        minMarkerCount,
+        maxMarkerCount,
         scanArgs.getLimit(),
         filterPolicy);
     return result;
@@ -513,6 +526,11 @@ private ScanResult scan(
       final DirectoryPolicy filterPolicy)
       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();
 
     // Mission Accomplished
diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerTool.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerTool.java
index 4c11a3389f..fc1abc19dd 100644
--- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerTool.java
+++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerTool.java
@@ -259,8 +259,25 @@ public void testRunAuditWithExpectedMarkers() throws Throwable {
         AUDIT,
         m(OPT_LIMIT), 0,
         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);
     expectMarkersInOutput(audit, expectedMarkersWithBaseDir);
   }