-
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-19351. S3A: Add config option to skip test with performance mode #7223
Conversation
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
PR looks good to me.
The S3A cloud connector team should chime in too.
I am assuming we can disable the performance code property in Ozone here? https://github.com/apache/ozone/blob/master/hadoop-ozone/integration-test/src/test/resources/contract/ozone.xml
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.
minor changes
/** | ||
* A property set to true if tests running in performance mode are enabled: {@value } | ||
*/ | ||
String KEY_PERFORMANCE_TESTS_ENABLED = TEST_FS_S3A + "perf.enabled"; |
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.
call this "performance.enabled"; perf is a bit too terse.
|
||
### Disabling tests running in performance mode | ||
|
||
Some tests running in performance mode turn off the safety checks. They expect breaking posix semantics. |
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.
"they expect operations which break POSIX semantics to succeed"
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.
Looks good to me overall pending minor comments by Steve.
🎊 +1 overall
This message was automatically generated. |
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.
looks good, just a doc change
For stores with stricter semantics, these test cases must be disabled. | ||
```xml | ||
<property> | ||
<name>test.fs.s3a.perf.enabled</name> |
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.
move to performance
here too
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
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.
LGTM
+1
ok, merged. if you cherrypick and retest against branch-3.4, and put up a PR of the backport, I will merge there too. |
Description of PR
Stores with posix semantics can't overwrite tests. Some tests will fail in performance mode. Add a configuration option to skip running tests in performance mode.
How was this patch tested?
Tested in ap-southeast-2.
Tested against Ozone FSO bucket with
test.fs.s3a.perf.enabled
set to false.For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?