HADOOP-16721. Improve S3A rename resilience

[steveloughran]

S3A Rename to

1. switch from verifying dest parent path is present and a directory to simply verifying that it isn't a file.
2. avoids race condition where thread 1 has deleted the last subdir/file but not yet recreated a parent marker
3. don't downgrade FileNotFoundException to 'false' if source doesn't exist
4. raise FileAlreadyExistsException if dest path exists or parent is a file.

If thread/process 1 deleted the subdir dest/subdir1 one and there
are no sibling subdirectories, then then the dir dest would not
exist until maybeCreateFakeParentDirectory() had performed a
LIST and, if needed, a PUT of a marker.
This creates a window where thread/process 2, trying to rename staging/subdir2
into dest could fail "parent does not exist".

And guess what:

1. Hive still thinks renaming directories from staging to the production dataset is a good idea.
2. It can spawn many threads to do this parallel (maybe even to compensate for the slow rename())
   performance of S3.
3. On a sufficiently overloaded system (lots of threads already doing the parallel filenames)
   the window of parent-dir-not-found can last long enough for things to fail.

Prior to S3 being consistent this wouldn't have been an issue

• rename()'s need to list everything underneath the source dir wasn't safe to use with delayed list consistency
• so S3Guard was effectively mandatory.
• and delayed LIST consistency may even have reduced the window size.

The fix: go from verifying parent dir exists to simply making sure that it isn't a file
is a weakening of the requirement "parent dir must exist" -but file:// already doesn't
require that.

Consequences

1. You can rename under a path which doesn't exist. file:// lets you do that already; contract tests have a switch for it.
   (FS contract test has switch for this)
2. Exceptions get raised when false would be returned and hive lose details of what went wrong. This
   isn't consistent with HDFS, but is with other stores (FS contract test has switch for this)
3. You can rename a few layers under a file, but then create() lets you do that already,
   and nobody has noticed in production.

If directory marker retention was enabled there's more likelihood that an empty dir marker existed -if it did then the race condition wouldn't exist. But as there are no guarantees that the marker will be there, that's not safe to rely on.

[steveloughran]

Testing with -Dparallel-tests -DtestsThreadCount=8 -Dmarkers=keep -Dscale

If the store is set to raise exceptions, a lot of tests which expect rename(bad options) to return false now get exceptions. One of the contract tests would downgrade if the raised exception was FileAlreadyExistsException and the contract xml said that was ok. I'm reluctant to go with a bigger patch.

this PR is so that Hive and friends can get better reporting on errors, rather than have them lost. It will be optional

[steveloughran]

Testing: s3 london with markers==keep and delete, s3guard on and off.

• change in error reporting of s3a FS matched with relevant changes in s3a.xml for contract tests.
• skip tests verifying that you can't rename 2+ levels under a file.
• Failures related to endpoints of common-crawl and ITestAssumeRole.testAssumeRoleBadInnerAuth: known and fixed in HADOOP-17511. Add audit/telemetry logging to S3A connector #2675.

[iwasakims]

LGTM overall. Tests against Tokyo region worked. I just left comments for nits.

$ mvn verify -Dtest=x -Dit.test=ITestS3AContractRename,ITestlRenameDeleteRace -Ds3guard
...
[INFO] Running org.apache.hadoop.fs.s3a.impl.ITestlRenameDeleteRace
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.044 s - in org.apache.hadoop.fs.s3a.impl.ITestlRenameDeleteRace
[INFO] Running org.apache.hadoop.fs.s3a.ITestS3GuardListConsistency
[INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 35.603 s - in org.apache.hadoop.fs.s3a.ITestS3GuardListConsistency
[INFO] Running org.apache.hadoop.fs.contract.s3a.ITestS3AContractRename
[WARNING] Tests run: 22, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 12.523 s - in org.apache.hadoop.fs.contract.s3a.ITestS3AContractRename

[iwasakims]

I got error on ITestS3AContractDistCp but this seems not to be related. I can not reproduce the error by running the ITestS3AContractDistCp alone by -Dit.test=ITestS3AContractDistCp.

$  mvn verify -Dtest=x -Dit.test=ITestS3AContract*
...
[INFO] Running org.apache.hadoop.fs.contract.s3a.ITestS3AContractDistCp
[ERROR] Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 47.658 s <<< FAILURE! - in org.apache.hadoop.fs.contract.s3a.ITestS3AContractDistCp
[ERROR] testNonDirectWrite(org.apache.hadoop.fs.contract.s3a.ITestS3AContractDistCp)  Time elapsed: 4.055 s  <<< FAILURE!
java.lang.AssertionError: Expected 2 renames for a non-direct write distcp expected:<2> but was:<0>
        at org.apache.hadoop.fs.contract.s3a.ITestS3AContractDistCp.testNonDirectWrite(ITestS3AContractDistCp.java:98)

[iwasakims]

3. don't downgrade FileNotFoundException to 'false' if source doesn't exist
4. raise FileAlreadyExistsException if dest path exists or parent is a file.

Should this JIRA be marked as incompatible for applicatinos assuming existing behavior?

[steveloughran]

testing: s3 london, most recently with -Dparallel-tests -DtestsThreadCount=6 -Dmarkers=keep -Ds3guard -Ddynamo -Dscale

[iwasakims]

+1. The ITestRenameDeleteRace failed without the fix of S3AFileSystem as expected. The test passed with the fix regardless of -Ds3guard. Thanks, @steveloughran.

[steveloughran]

ah, thanks -lovely. just tweaked the imports slightly for better backporting; I'll merge once the next compile is good, then do a cp and retest for branch-3.3.