[
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]