-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-18508. S3A: Support parallel integration test runs on same bucket #5081
HADOOP-18508. S3A: Support parallel integration test runs on same bucket #5081
Conversation
s3 london
where mvit is mvn verify with some options need to run a few more times to make sure all files are created in isolation. then kick off two test runs in different source trees at the same time |
💔 -1 overall
This message was automatically generated. |
409f306
to
d43c91d
Compare
rebased; running as an experiment. the jobid stuff is all set up in the pom.xml. i actually want to be able to set it in the auth-keys.xml file, as that would let me set up different source trees testing against different parts of the same store. proposed: support it as a sysprop or a config option |
d43c91d
to
c4ec4ad
Compare
c4ec4ad
to
a37254e
Compare
a37254e
to
e142a67
Compare
e142a67
to
29386a6
Compare
🎊 +1 overall
This message was automatically generated. |
Can I chase @mukund-thakur @HarshitGupta11 @ahmarsuhail for reviews here. This is so we can run multiple CI tests against the same bucket in parallel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need something similar for ABFS?
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/testing.md
Outdated
Show resolved
Hide resolved
@Test | ||
@Override | ||
public void testWDAbsolute() throws IOException { | ||
Path absoluteDir = getTestRootPath(fSys, "test/existingDir"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please explain this test a bit more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
override of the superclass. just needed a test relative path.
I've cut this and fixed the base class; other subclasses copy this fix and it is harmless on others
* @throws IOException failure | ||
*/ | ||
@Override | ||
protected void deleteTestDirInTeardown() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for the huge files test, we will have to delete manually via the S3 lifecycle rule as mentioned in the testting.doc?
This is just to reduce the time taken by tests right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we delete the dir in test_800_DeleteHugeFiles(), at the end of the entire sequence of operations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is only for the huge file. the testPath still remains right?
Are you running multiple parallel tests in different terminal windows to verify everything is correct? |
🎊 +1 overall
This message was automatically generated. |
ooh, good point. I did a while back, but let me kick that off again |
...and I've found out that terasort paths aren't isolated
|
🎊 +1 overall
This message was automatically generated. |
checkstyles where I changed the base contract tests
|
🎊 +1 overall
This message was automatically generated. |
..it and the STS one are both with jiras. STS failure depends on the region you are running from, so it is test setup. |
@mukund-thakur given the failures you saw were just related to you not reading the docs, can we merge this? |
|
let me rebase and look at the failing test before I merge. You are only seeing it on this branch? Draft message for the commit
|
…et in parallel * adds job id to fork id * adds option to disable root tests * documents this * gets more tests working under the job/fork subdir rather than absolute paths. tracking them down is tricky. Change-Id: I95530f883307b66971c0300631993798967b6739
Change-Id: Ieb9b5bbb0644c9e030dbdc80f51a032964e727ab
Change-Id: If00e579a831110886e0e1ade46fe72dbe70269fa
* cut mention of cloudstore sessionkeys * review changes and cut back on diff Change-Id: Ib2bdd02921f7cdbf9e0b924cab0bfbf30f21c395
Change-Id: I48fc6b30ba69a616fae688c45dffacc5da504014
* terasort uses path under fork/job id, which requires disabling test dir cleanup. * +method name for logs includes memory option * +reports CSV file includes memory flag * sequential and surefire test runs also pass down job id, so that it is possible to safely run a single test in one process while a full suite runs against the same bucket. Change-Id: Ib37ef6b5f23e5e7beb53d7c04c44e811ac4e2fa3
Change-Id: Ia2aa6e793a02914f2e391ad699ab998975f1a982
…ileSystem I thought I'd already cleaned them, but yetus was still unhappy. cleaned up the test even more to see if that helps Change-Id: If4096e70e7921db8790fb56351077840cb4317d9
23e696c
to
d82fa09
Compare
-always create base dir at the start of each test -use methodPath() -remove final relics of s3guard comments/qualifiers -remove parameterization on directory marker retention, the default of "keep" stresses the trickiest bit of the code as it leaves more markers around. Change-Id: Ib1fa2940116170bd5d00375ab51ed582f8cad1f6
@mukund-thakur couldn't replicate the failure but I'd disabled the tests during some s3express testing and not-reenabled it. still couldn't replicate it. and when you skip root tests then it's also disabled as mixing SSE-C really causes problems. Or at least used to. Anyway, I've made sure the suite sets a base dir at the start of each methodPath() and builds each sub path off it, whatever problem you encountered seems to have been related. I also cut out the markers=delete run phase as its markers=keep where more problems surface...this saves 30s off the test suite. Which makes me think: we should review when we use markers=delete and other than for some compatibility tests, remove them all. What do you think about having more time in your life back? |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Failing test ITestS3AEncryptionSSEC succeeded after your change. So good to merge pending yetus. +1 And I would for sure want some more time in my life back. |
Change-Id: I18a0bd833f302cbcc4676987647a6ce3ec731a8e
surely you mean "some more time to work on JIRAs" |
💔 -1 overall
This message was automatically generated. |
Change-Id: Ia340d186e142eeadec3b093edb2d14a30c2e5197
🎊 +1 overall
This message was automatically generated. |
in trunk; testing in 3.3.4 and will push up there. |
…ket (#5081) It is now possible to provide a job ID in the maven "job.id" property hadoop-aws test runs to isolate paths under a the test bucket under which all tests will be executed. This will allow independent builds *in different source trees* to test against the same bucket in parallel, and is designed for CI testing. Example: mvn verify -Dparallel-tests -Droot.tests.enabled=false -Djob.id=1 mvn verify -Droot.tests.enabled=false -Djob.id=2 - Root tests must be be disabled to stop them cleaning up the test paths of other test runs. - Do still regularly run the root tests just to force cleanup of the output of any interrupted test suites. Contributed by Steve Loughran
…ket (apache#5081) It is now possible to provide a job ID in the maven "job.id" property hadoop-aws test runs to isolate paths under a the test bucket under which all tests will be executed. This will allow independent builds *in different source trees* to test against the same bucket in parallel, and is designed for CI testing. Example: mvn verify -Dparallel-tests -Droot.tests.enabled=false -Djob.id=1 mvn verify -Droot.tests.enabled=false -Djob.id=2 - Root tests must be be disabled to stop them cleaning up the test paths of other test runs. - Do still regularly run the root tests just to force cleanup of the output of any interrupted test suites. Contributed by Steve Loughran
…ket (apache#5081) It is now possible to provide a job ID in the maven "job.id" property hadoop-aws test runs to isolate paths under a the test bucket under which all tests will be executed. This will allow independent builds *in different source trees* to test against the same bucket in parallel, and is designed for CI testing. Example: mvn verify -Dparallel-tests -Droot.tests.enabled=false -Djob.id=1 mvn verify -Droot.tests.enabled=false -Djob.id=2 - Root tests must be be disabled to stop them cleaning up the test paths of other test runs. - Do still regularly run the root tests just to force cleanup of the output of any interrupted test suites. Contributed by Steve Loughran
How was this patch tested?
running the forked tests with different job ids, disabling root dir cleanup and seeing what was created in separate subdirs
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?