You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Aaron Fabbri (JIRA)" <ji...@apache.org> on 2017/11/08 05:27:00 UTC

[jira] [Comment Edited] (HADOOP-15003) Merge S3A committers into trunk: Yetus patch checker

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

Aaron Fabbri edited comment on HADOOP-15003 at 11/8/17 5:26 AM:
----------------------------------------------------------------

{quote}That's not a minor failure though{quote}
I was assuming this is a test code failure, not logic issues in core S3A or even in the "experimental" magic committer codepath, based on the rest of the testing I've done.

(edit: Looks like the {{@After}} deletes /test, not /tests3aScale.  So much for that theory.  Shortening this a bit)

Curious this test case {{ITestHugeMagicCommits}} always succeeds unless I turn on parallel tests.  It looks like your excluding the test from parallel execution. 

{noformat}
<exclude>**/ITestS3AHuge*.java</exclude>
<exclude>**/ITestHuge*.java</exclude>
{noformat}

The ITestHugeMagicCommits thing is a complex way to get this test coverage of S3A's handling of magic paths on create. I spent some time earlier reading through it.

Though we depend on the ordering of test cases to make them stateful (understood considering the overhead associated with setting up the huge files), we still have {{@Before}} and {{@After}} functions which do a bunch of stuff between the ordered test cases.  Things happening in the four layers of {{@Before}} overrides, from super to sub class, roughly:

- creating fs contract 
- getting test filesystem from the contract  *Couldn't this purge multiparts and break this test case?*  I guess if someone sets a very low age value in their config.
- mkdirs(testPath)
- testPath = path("/tests3ascale") in S3AScaleTestBase.. which defines getter getTestPath()
- assume scale tests are enabled
- blah blah
- set up some paths for the {{__magic}} directory in ITestHugeMagicCommits
 
Now after each test case, looking at {{@After}} functions:
- *Delete test dir*, recursively.  Looking at logs, deletes /test, not /tests3ascale, though.
- fs.close() 

Spending a little time looking at debug logs after dinner I'll shout if I get more detail.


was (Author: fabbri):
{quote}That's not a minor failure though{quote}
I was assuming this is a test code failure, not logic issues in core S3A or even in the "experimental" magic committer codepath, based on the rest of the testing I've done.

*TL;DR* It seems like {{@After}} is deleting everything between test cases.

Going to brainstorm a bit here. Curious this test case {{ITestHugeMagicCommits}} always succeeds unless I turn on parallel tests.  It looks like your excluding the test from parallel execution. According to pom comment this is based on concern for multipart purging on FS init (which is moot here I believe).

{noformat}
<exclude>**/ITestS3AHuge*.java</exclude>
<exclude>**/ITestHuge*.java</exclude>
{noformat}

The ITestHugeMagicCommits thing is a complex way to get this test coverage of S3A's handling of magic paths on create.  We subclass {{AbstractSTestS3AHugeFiles}}, which uses {{@FixMethodOrder(MethodSorters.NAME_ASCENDING)}} to ensure your test functions run in a certain order.  {{AbstractSTestS3AHugeFiles}} in turn subclasses {{S3AScaleTestBase}} which subclasses {{AbstractS3ATestBase}} which then subclasses {{AbstractFSContractTestBase}}.  (Aside: composition > inheritance IMO)

Though we depend on the ordering of test cases to make them stateful (understood considering the overhead associated with setting up the huge files), we still have {{@Before}} and {{@After}} functions which do a bunch of stuff between the ordered test cases.  Things happening in the four layers of {{@Before}} overrides, from super to sub class, roughly:

- creating fs contract 
- getting test filesystem from the contract (implementation: somewhere in the inheritance graph above).  *Couldn't this purge multiparts and break this test case?*
- mkdirs(testPath)
- testPath = path("/tests3ascale") in S3AScaleTestBase.. which defines getter getTestPath()
- assume scale tests are enabled
- blah blah
- set up some paths for the {{__magic}} directory in ITestHugeMagicCommits
 
Now after each test case, looking at {{@After}} functions:
- *Delete test dir*, recursively.  *Why doesn't this break the test, even in serial mode?*
- fs.close()

Some things we're depending on here.. off top of my head.

- {{@FixMethodOrder(MethodSorters.NAME_ASCENDING)}} applies to methods of all subclasses.
- I'm actually running JUnit 4.11
- Behavior of above is same w/ and w/o parallel tests profile.

Any other ideas as to why this only fails in parallel mode?  I feel like I'm missing something obvious.


> 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
>
>
> 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: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org