You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by st...@apache.org on 2020/11/23 21:49:57 UTC

[hadoop] branch branch-3.3 updated: HADOOP-17332. S3A MarkerTool -min and -max are inverted. (#2425)

This is an automated email from the ASF dual-hosted git repository.

stevel pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/branch-3.3 by this push:
     new 38cc47d  HADOOP-17332. S3A MarkerTool -min and -max are inverted. (#2425)
38cc47d is described below

commit 38cc47d30897086db0e146eb48f0014e0dd80cbf
Author: Steve Loughran <st...@cloudera.com>
AuthorDate: Mon Nov 23 20:49:42 2020 +0000

    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
    
    Change-Id: I15c40863f0db0675c7d60db477cb3bf1693cae49
---
 .../org/apache/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 f817223..c1e4c8a 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
@@ -36,6 +36,7 @@ import com.amazonaws.AmazonClientException;
 import com.amazonaws.services.s3.model.DeleteObjectsRequest;
 import com.amazonaws.services.s3.model.MultiObjectDeleteException;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -395,10 +396,22 @@ public final class MarkerTool extends S3GuardTool {
     } 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 @@ public final class MarkerTool extends S3GuardTool {
       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 4c11a33..fc1abc1 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 class ITestMarkerTool extends AbstractMarkerToolTest {
         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);
   }


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org