[ 
https://issues.apache.org/jira/browse/HADOOP-15003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16248034#comment-16248034
 ] 

Steve Loughran commented on HADOOP-15003:
-----------------------------------------

Aarons test failure has forced me to look at all the abort code in the 
committers. So I have.


* Found bug in the existing S3AFileSystem.listPendingUploads(path), in that as 
path never had a trailing "/", it'd list pending uploads in any directory which 
began with the same prefix, e.g. /work/job1 and work/job12. The final committer 
cleanup() logic would therefore abort adjacent work. Fixed. 
* turned off that bulk cleanup to see what would happen, helped explore that 
list and delete pending code, which ultimately proved insufficent (see below). 
listPendingUploads-> abort is in fact the only way to do it reliably.
* logging improved @debug level, with a bit more on impoort to deal with the 
use case "things aren't working in production, here are the logs, VM is gone"

  
Two little issues related to cleanup
  

Magic App attempt abort would list all in app attempt 
dest/__magic/$jobid_$attemptid/{*.pendingset, *.pending} and clean up
But this doesn't cleanup on any failed attempt, which will have a different 
attemptID, so not be found. 

For the magic committer, switched to a much simpler cleanup of
* list all uploads pending under $dest, abort
* magic: delete __magic/*
* staging: abort wrapped job
* staging: delete local staging dirs for entire job

That is: no list/read/cancel  of pending job & task events.  Expensive *and not 
complete*. 
This bulk abort of all pending uploads of a  dest handles: failed tasks, failed 
attempts, failure of incomplete jobs  from other processes/work which was never 
aborted. (e.g: a previous query  somewhere failed leaving pending work). 
This was already being done after all the "structured"  aborts; here I remove 
that work and only do the bulk list & rm for a  simpler life; made sure all of 
that excutes in parallel for fastest cleanup.

This significantly simplifies the abort, commit failure and helper classes (no 
need to suppress exceptions),
and reduces the diff between the staging & magic committers (so more has been 
pulled up to the base class)

That is: this is the first patch for a while which cuts out production code in 
any measurable way.

The staging committer *currently* does the same, but it retains the listing and 
abort of all outstanding requests first. Why? I've kept it there to handle the 
use case of >1 partitioned commit running simultaneously, where you don't want 
to abort all the outstanding stuff. That's not fully rounded off in this patch 
because it's still doing the bulk cleanup: I want to see what Ryan thinks is 
best here. I think I'm going to opt for: list then optional bulk abort. That 
means the default doesn't leave pending objects around. That makes the list 
operation superfluous, of course, but (a) it's going against HDFS so is less 
expensive and (b) means that all tests cover the codepath, rather than having a 
condition which has weak coverage. That is: I don't think it's that harmful: 
there's the cost of an HDFS list(recursive=true), an open + parse of every 
read; For the magic committer, in constrast, its more expensive (the LIST, the 
GET) and it doesn't support that notion of partitioned work: you can't get >1 
job writing to the same directory tree.


h3. Docs: 

* mention examining the _SUCCESS file in troubleshooting section
* defaults of the various options

I'm wrapping up some details on cleanup in the committer arch doc, not included 
in this patch as it is late on a friday evening.

h3. Misc

* Invoke adds new method {{ignoreIOExceptions(Log, text, path, 
lambda-operation)}} which runs the operation & logs & info if there is a 
problem. This replaces a lot of the try/catch(IOE ignored) sequences in 
cleanup. What it makes easy is to isolate every single operation this way, so 
if one fails, the next step runs. This makes cleanup potentially more rigorous.
* S3AUtils {{list(fs, path, recursive, filter)}}, replicates the classic 
{{FileSystem.list(path, filter)}} but works on the higher performance recursive 
listFileStatus operation, and returns a list of those. Some bits of code taking 
{{List<FileStatus>}} have had to be changed to {{List<? extends FileStatus>}} 
because despite Java type erasure means this is the same at runtime, it still 
complains needlessly in the compile.
  

h3. Testing

* Huge magic commit test is called ITestS3AHugeMagicCommits; guarantees 
excluded with the normal rules. This is important as the test_010_ test case, 
which creates the file, skips the operation if it runs in parallel...which is 
going to fail the test with *the exact stack trace which aaron saw*. That is, I 
think the test failure really could have been a false alarm. But it forced me 
to look at cleanup, which is a good thing.
* New test to create two parallel jobs and work each one simultaneously. Found 
the listMultipartUploads-prefix bug.

* The mock tests got very confused; the mock AmazonS3Client now has to track 
outstanding MPU requests so that listMultipartUploads() can return the full 
list of active requests, and abortMultipartUpload() raises 404 if there isn't 
one outstanding. All the little mock answers are now java-8 lambda expressions 
to make a little bit smaller. Reinstated a comment out bit of 
{{assertValidUpload}} which checked expected tag list vs. actual; I must have 
commented it out a while back.


h3. Test run

Test runs: Endpoint S3 ireland with ddb on


One transient failure where my explicitly enabled throttling failed too often 
for the test; Cranked back the normal throttle there from 50% to 25%.

{code}
Tests run: 16, Failures: 0, Errors: 1, Skipped: 1, Time elapsed: 101.795 sec 
<<< FAILURE! - in org.apache.hadoop.fs.s3a.commit.ITestCommitOperations
testCommitEmptyFile(org.apache.hadoop.fs.s3a.commit.ITestCommitOperations)  
Time elapsed: 12.755 sec  <<< ERROR!
org.apache.hadoop.fs.s3a.AWSServiceThrottledException: Completing multipart 
commit on fork-0007/test/DELAY_LISTING_ME/testCommitEmptyFile/empty-commit.txt: 
com.amazonaws.AmazonServiceException: throttled count = 3 (Service: null; 
Status Code: 503; Error Code: null; Request ID: null):null: throttled count = 3 
(Service: null; Status Code: 503; Error Code: null; Request ID: null)
        at 
org.apache.hadoop.fs.s3a.InconsistentAmazonS3Client.maybeFail(InconsistentAmazonS3Client.java:611)
        at 
org.apache.hadoop.fs.s3a.InconsistentAmazonS3Client.completeMultipartUpload(InconsistentAmazonS3Client.java:537)
        at 
org.apache.hadoop.fs.s3a.WriteOperationHelper.lambda$finalizeMultipartUpload$1(WriteOperationHelper.java:226)
        at org.apache.hadoop.fs.s3a.Invoker.once(Invoker.java:108)
        at org.apache.hadoop.fs.s3a.Invoker.lambda$retry$3(Invoker.java:259)
        at org.apache.hadoop.fs.s3a.Invoker.retryUntranslated(Invoker.java:313)
        at org.apache.hadoop.fs.s3a.Invoker.retry(Invoker.java:255)
        at 
org.apache.hadoop.fs.s3a.WriteOperationHelper.finalizeMultipartUpload(WriteOperationHelper.java:219)
        at 
org.apache.hadoop.fs.s3a.WriteOperationHelper.completeMPUwithRetries(WriteOperationHelper.java:264)
        at 
org.apache.hadoop.fs.s3a.commit.CommitOperations.innerCommit(CommitOperations.java:166)
        at 
org.apache.hadoop.fs.s3a.commit.CommitOperations.commit(CommitOperations.java:138)
        at 
org.apache.hadoop.fs.s3a.commit.CommitOperations.commitOrFail(CommitOperations.java:121)
        at 
org.apache.hadoop.fs.s3a.commit.ITestCommitOperations.commit(ITestCommitOperations.java:325)
        at 
org.apache.hadoop.fs.s3a.commit.ITestCommitOperations.commit(ITestCommitOperations.java:302)
        at 
org.apache.hadoop.fs.s3a.commit.ITestCommitOperations.createCommitAndVerify(ITestCommitOperations.java:284)
        at 
org.apache.hadoop.fs.s3a.commit.ITestCommitOperations.testCommitEmptyFile(ITestCommitOperations.java:202)
{code}


> Merge S3A committers into trunk: Yetus patch checker
> ----------------------------------------------------
>
>                 Key: HADOOP-15003
>                 URL: https://issues.apache.org/jira/browse/HADOOP-15003
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>    Affects Versions: 3.0.0
>            Reporter: Steve Loughran
>            Assignee: Steve Loughran
>         Attachments: HADOOP-13786-041.patch, HADOOP-13786-042.patch, 
> HADOOP-13786-043.patch, HADOOP-13786-044.patch, HADOOP-13786-045.patch, 
> HADOOP-13786-046.patch, HADOOP-13786-047.patch
>
>
> This is a Yetus only JIRA created to have Yetus review the 
> HADOOP-13786/HADOOP-14971 patch as a .patch file, as the review PR 
> [https://github.com/apache/hadoop/pull/282] is stopping this happening in 
> HADOOP-14971.
> Reviews should go into the PR/other task



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to