-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19348. Integrate analytics accelerator into S3A. #7433
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
Conversation
|
💔 -1 overall
This message was automatically generated. |
944fbbb to
a8da9b2
Compare
|
ok, squash didn't take either |
|
build was terminated after 12 hours starts ends raise on common-dev, assume it's a yetus/config/timeout issue. |
steveloughran
left a comment
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 pending that yetus. I don't want to ignore it as it may not be a transient failure, just something that tips yetus over the edge.
|
weird that this one is also checking out: |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
steveloughran
left a comment
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.
- spelling in variable name to fix
- test tuning with 1 extra assert
+1 pending these
| isCSEEnabled = CSEUtils.isCSEEnabled(getS3EncryptionAlgorithm().getMethod()); | ||
|
|
||
| isAnalyticsAccelaratorEnabled = StreamIntegration.determineInputStreamType(conf).equals(InputStreamType.Analytics); | ||
| isAnalyticsAccelaratorEnabled = StreamIntegration.determineInputStreamType(conf) |
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.
nit: spelling
|
|
||
| verifyStatisticCounterValue(ioStats, STREAM_READ_ANALYTICS_OPENED, 1); | ||
| } | ||
| verifyStatisticCounterValue(ioStats, STREAM_READ_ANALYTICS_OPENED, 1); |
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.
add a check for the filesystem iostats too, to make sure it trickles up
|
💔 -1 overall
This message was automatically generated. |
|
|
||
| verifyStatisticCounterValue(ioStats, STREAM_READ_ANALYTICS_OPENED, 1); | ||
|
|
||
| verifyStatisticCounterValue(getFileSystem().getIOStatistics(), |
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.
@steveloughran can't get this one to work..
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.
Ok think it's to do with the test base class maybe, and these one's don't aggregate to the FS. I added an assertion in ITestS3AOpenCost which extends AbstractS3ACostTest and can see it get's propagated.
s3a.AbstractS3ATestBase (AbstractS3ATestBase.java:dumpFileSystemIOStatistics(130)) - Aggregate FileSystem Statistics counters=((action_file_opened=1)
(action_http_head_request=3)
(audit_request_execution=13)
....
(stream_read_analytics_opened=1)
....
Think this is enough for now, and I'll remove this assertion and merge as is. Once we support statistics properly, we can add in tests in ITestS3AOpenCost
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Initial support for Analytics Accelerator Library, which provides a new parquet aware input stream. Contributed by: Ahmar Suhail Reviewed by: Steve Loughran
Description of PR
Copy of #7334, checking if a new PR from a new branch keeps yetus happy.
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?