HADOOP-19278. Final pre-merge review

* yetus complaints
* search source for "marker" and identify what needs to change
  (docs, some test comments and messages)
* same for "keeping"

Change-Id: I78988d33a15be2fbc48c7bc309391f8a7e52f63c

Author: steveloughran

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java

@@ -869,10 +869,13 @@ private Constants() {
   /**
    * Paths considered "authoritative".
    * When S3guard was supported, this skipped checks to s3 on directory listings.
-   * It is also use to optionally disable marker retentation purely on these
-   * paths -a feature which is still retained/available.
+   * It was also possilbe to use to optionally disable marker retentation purely on these
+   * paths -a feature which is no longer available.
+   * As no feature uses this any more, it is declared as deprecated.
    * */
+  @Deprecated
   public static final String AUTHORITATIVE_PATH = "fs.s3a.authoritative.path";
+  @Deprecated
   public static final String[] DEFAULT_AUTHORITATIVE_PATH = {};
 
   /**
@@ -1405,6 +1408,7 @@ private Constants() {
    * Always false.
    * Value: {@value}.
    */
+  @Deprecated
   public static final String STORE_CAPABILITY_DIRECTORY_MARKER_POLICY_DELETE
       = "fs.s3a.capability.directory.marker.policy.delete";
 
@@ -1414,6 +1418,7 @@ private Constants() {
    * This probe always returns false.
    * Value: {@value}.
    */
+  @Deprecated
   public static final String
       STORE_CAPABILITY_DIRECTORY_MARKER_POLICY_AUTHORITATIVE =
       "fs.s3a.capability.directory.marker.policy.authoritative";

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/MkdirOperation.java

@@ -262,9 +262,6 @@ S3AFileStatus probePathStatus(Path path,
     /**
      * Create a fake directory, always ending in "/".
      * Retry policy: retrying; translated.
-     * the keepMarkers flag controls whether or not markers
-     * are automatically kept (this is set when creating
-     * directories under a magic path, always)
      * @param dir dir to create
      * @throws IOException IO failure
      */

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/assumed_roles.md

@@ -295,13 +295,10 @@ views or operations.
 Particular troublespots are "directory markers" and
 failures of non-atomic operations, particularly `rename()` and `delete()`.
 
-A directory marker such as `/users/` will not be deleted if the user `alice`
-creates a directory `/users/alice` *and* she only has access to `/users/alice`.
-
-When a path or directory is deleted, the parent directory may not exist afterwards.
-In the example above, if `alice` deletes `/users/alice` and there are no
-other entries under `/users/alice`, then the directory marker `/users/` cannot
-be created. The directory `/users` will not exist in listings,
+If `alice` deletes `/users/alice` and there are no
+other entries under `/users/alice`, or a directory marker `/users` then that
+directory marker cannot be created.
+The directory `/users` will not exist in listings,
 `getFileStatus("/users")` or similar.
 
 Rename will fail if it cannot delete the items it has just copied, that is

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/directory_markers.md

@@ -12,7 +12,7 @@
   limitations under the License. See accompanying LICENSE file.
 -->
 
-# Controlling the S3A Directory Marker Behavior
+# S3A Directory Marker Behavior
 
 This document discusses directory markers and a change to the S3A
 connector: surplus directory markers are no longer deleted.
@@ -21,7 +21,7 @@ This document shows how the performance of S3 I/O, especially applications
 creating many files (for example Apache Hive) or working with versioned S3 buckets can
 increase performance by changing the S3A directory marker retention policy.
 
-This release always retains markers, 
+This release always retains markers
 and _is potentially not backwards compatible_ with hadoop versions
 released before 2021.
 
@@ -69,7 +69,7 @@ Everyone using these branches should upgrade to a supported version.
 
 ## History
 
-### Hadoop 3.3.1 Directory marker retention is optional 
+### Hadoop 3.3.1 Directory marker retention is optional
 
 [HADOOP-13230](https://issues.apache.org/jira/browse/HADOOP-13230)
  _S3A to optionally retain directory markers_
@@ -84,7 +84,7 @@ Marker deletion can still be enabled.
 
 Since this release there have been no reports of incompatibilities
 surfacing "in the wild". That is: out of date hadoop versions are not
-being used to work into the same parts of S3 buckets as modern releases. 
+being used to work into the same parts of S3 buckets as modern releases.
 
 ### Hadoop 3.5: markers are never deleted
 

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md

@@ -23,12 +23,12 @@
 
 ###  <a name="directory-marker-compatibility"></a> Directory Marker Compatibility
 
-This release does not delete directory markers when creating
+This release never delete directory markers when creating
 files or directories underneath.
 This is incompatible with versions of the Hadoop S3A client released
 before 2021.
 
-Consult [Controlling the S3A Directory Marker Behavior](directory_markers.html) for
+Consult [S3A and Directory Markers](directory_markers.html) for
 full details.
 
 ## <a name="documents"></a> Documents
@@ -40,7 +40,7 @@ full details.
 * [Working with Third-party S3 Stores](./third_party_stores.html)
 * [Troubleshooting](./troubleshooting_s3a.html)
 * [Prefetching](./prefetching.html)
-* [Controlling the S3A Directory Marker Behavior](directory_markers.html).
+* [S3A and Directory Markers](directory_markers.html).
 * [Auditing](./auditing.html).
 * [Committing work to S3 with the "S3A Committers"](./committers.html)
 * [S3A Committers Architecture](./committer_architecture.html)

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileOperationCost.java

@@ -45,10 +45,6 @@
 
 /**
  * Use metrics to assert about the cost of file API calls.
- * Parameterized on directory marker keep vs delete.
- * When the FS is instantiated with creation performance, things
- * behave differently...its value is that of the marker keep flag,
- * so deletion costs are the same.
  */
 public class ITestS3AFileOperationCost extends AbstractS3ACostTest {
 
@@ -111,7 +107,7 @@ public void testCostOfListFilesOnFile() throws Throwable {
 
   @Test
   public void testCostOfListFilesOnEmptyDir() throws Throwable {
-    describe("Perpforming listFiles() on an empty dir with marker");
+    describe("Performing listFiles() on an empty dir with marker");
     // this attem
     Path dir = path(getMethodName());
     S3AFileSystem fs = getFileSystem();

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java

@@ -246,12 +246,6 @@ public interface S3ATestConstants {
   Duration TEST_SESSION_TOKEN_DURATION = Duration.ofSeconds(
       TEST_SESSION_TOKEN_DURATION_SECONDS);
 
-  /**
-   * Test option to enable audits of the method path after
-   * every test case.
-   */
-  String DIRECTORY_MARKER_AUDIT = "fs.s3a.directory.marker.audit";
-
   /**
    * Constant bytes being written when Client side encryption KMS is enabled
    * for a test. This bytes written takes into account "EncryptionContext",

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java

@@ -1081,8 +1081,7 @@ public static List<Path> createDirsAndFiles(final FileSystem fs,
     List<CompletableFuture<Path>> futures = new ArrayList<>(paths.size()
         + dirs.size());
 
-    // create directories. With dir marker retention, that adds more entries
-    // to cause deletion issues
+    // create directories.
     try (DurationInfo ignore =
              new DurationInfo(LOG, "Creating %d directories", dirs.size())) {
       for (Path path : dirs) {

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/ITestCommitOperationCost.java

@@ -112,37 +112,12 @@ protected String fileSystemIOStats() {
     return ioStatisticsToPrettyString(getFileSystem().getIOStatistics());
   }
 
-  @Test
-  public void testMagicMkdir() throws Throwable {
-    describe("Mkdirs 'MAGIC PATH' always skips dir marker deletion");
-    S3AFileSystem fs = getFileSystem();
-    Path baseDir = methodPath();
-    // create dest dir marker, always
-    fs.mkdirs(baseDir);
-    Path magicDir = new Path(baseDir, MAGIC_PATH_PREFIX + JOB_ID);
-    verifyMetrics(() -> {
-      fs.mkdirs(magicDir);
-      return fileSystemIOStats();
-    },
-        with(OBJECT_BULK_DELETE_REQUEST, 0),
-        with(OBJECT_DELETE_REQUEST, 0),
-        with(DIRECTORIES_CREATED, 1));
-    verifyMetrics(() -> {
-      fs.delete(magicDir, true);
-      return fileSystemIOStats();
-    },
-        with(OBJECT_BULK_DELETE_REQUEST, 0),
-        with(OBJECT_DELETE_REQUEST, 1),
-        with(DIRECTORIES_CREATED, 0));
-    assertPathExists("parent", baseDir);
-  }
-
   /**
    * When a magic subdir is deleted, parent dirs are not recreated.
    */
   @Test
   public void testMagicMkdirs() throws Throwable {
-    describe("Mkdirs __magic_job-<jobId>/subdir always skips dir marker deletion");
+    describe("Mkdirs __magic_job-<jobId>/subdir always skips dir marker recreation");
     S3AFileSystem fs = getFileSystem();
     Path baseDir = methodPath();
     Path magicDir = new Path(baseDir, MAGIC_PATH_PREFIX + JOB_ID);
@@ -192,7 +167,7 @@ private void abortActiveStream() throws IOException {
 
   @Test
   public void testCostOfCreatingMagicFile() throws Throwable {
-    describe("Files created under magic paths skip existence checks and marker deletes");
+    describe("Files created under magic paths skip existence checks");
     S3AFileSystem fs = getFileSystem();
     Path destFile = methodSubPath("file.txt");
     fs.delete(destFile.getParent(), true);
@@ -213,9 +188,8 @@ public void testCostOfCreatingMagicFile() throws Throwable {
 
       stream.write("hello".getBytes(StandardCharsets.UTF_8));
 
-      // when closing, there will be no directories deleted
-      // we do expect two PUT requests, because the marker and manifests
-      // are both written
+      // when closing, we expect two PUT requests,
+      // because the marker and manifests are both written
       LOG.info("closing magic stream to {}", magicDest);
       verifyMetrics(() -> {
         stream.close();

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/performance/ITestDirectoryMarkerListing.java

@@ -402,10 +402,6 @@ public void testDelete() throws Throwable {
 
   /**
    * Rename the base directory, expect the source files to move.
-   * <p></p>
-   * Whether or not the marker itself is copied depends on whether
-   * the release's rename operation explicitly skips
-   * markers on renames.
    */
   @Test
   public void testRenameBase() throws Throwable {
@@ -433,14 +429,8 @@ public void testRenameBase() throws Throwable {
     assertIsFile(destMarkerPeer);
     head(destFileKeyUnderMarker);
 
-    // probe for the marker based on expected rename
-    // behavior
-    if (RENAME_COPIES_MARKERS) {
-      head(destMarkerKeySlash);
-    } else {
-      head404(destMarkerKeySlash);
-    }
-
+    // rename doesn't copy non-leaf markers
+    head404(destMarkerKeySlash);
   }
 
   /**