-
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-18373. IOStatisticsContext tuning #4705
Conversation
Tested against
Errors:
|
🎊 +1 overall
This message was automatically generated. |
For
Tried with both buckets, with and without ACL enabled.
This doesn't seem relevant to the patch but wondering what I am missing in the configs specifically for |
8c5d530
to
127d230
Compare
🎊 +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. i think I'd prefer an enabled()
as the public predicate
@@ -178,6 +178,12 @@ | |||
<value>true</value> | |||
</property> | |||
|
|||
<!-- Enable IOStatisticsContext support for Thread level. --> |
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.
mixed feelings here. hive team and others who serialize configs don't like us overfilling the xml file with defaults
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 might not have issue with test resource xml changes 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.
no issues there at all
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.
lets go with it
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java
Outdated
Show resolved
Hide resolved
...project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/IOStatisticsContext.java
Outdated
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
never seen that; docs say "AWS access key ID provided does not exist in our records." it might be that the arn of the token you are asking for doesn't exist, or that you don't have permissions to create sessions for it and it is failing |
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.
+1
merged to trunk; testing branch-3.3 and will push up if all is good |
The name of the option to enable/disable thread level statistics is "fs.iostatistics.thread.level.enabled"; There is also an enabled() probe in IOStatisticsContext which can be used to see if the thread level statistics is active. Contributed by Viraj Jasani
Thanks @steveloughran. Here is what I did: created role, provided policy, created user, provided the same policy. Updated role's trust relationship to allow user to perform assume-role on the role.
Perhaps the user (who did the assume-role) doesn't have some specific permission? |
I was able to figure out the issue, please ignore above comment. |
what was it? test running as other user? fwiw i use a very restricted user/role for my s3 tests, so if that key ever leaked, it would limit the damage to accessing my test s3 buckets, assume one role, etc. not even start an EC2 vm |
Basically for this test, I manually did the
Ah yes, this is definitely very good suggestion, thanks! |
yeah, you can't ask for session creds with session creds cloudstore has a command to ask for session credentials with your full s3a secrets, prints as env vars, properties, xml handy for getting some secrets into a vm for a while |
Thank you for sharing, Steve! |
The name of the option to enable/disable thread level statistics is "fs.iostatistics.thread.level.enabled"; There is also an enabled() probe in IOStatisticsContext which can be used to see if the thread level statistics is active. Contributed by Viraj Jasani
Description of PR
Tuning of the IOStatisticsContext code
change property name to "fs.iostatistics...." for consistent naming
Edit core-site.xml to always collect context iOStatistics