-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-18507. VectorIO FileRange type to support a "reference" field #5076
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-18507. VectorIO FileRange type to support a "reference" field #5076
Conversation
Add new reference field; verifies it remains in merging. also fix up those tests which used string comparison of range values into actual field checking. Change-Id: I46fe5fb9d21f58a15bce0b42bd79cefd1bd0a0fe
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. Yetus failed but it doesn't show any error.
Also did you re-ran the vectored io integration tests? Which endpoint?
| * Get any reference passed in to the file range constructor. | ||
| * This is not used by any implementation code; it is to help | ||
| * bind this API to libraries retrieving multiple stripes of | ||
| * dat in parallel.j |
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.
typo
| FileRange.createFileRange(1000, 100) | ||
| FileRange.createFileRange(3000, 100, "1"), | ||
| FileRange.createFileRange(2100, 100, "2"), | ||
| FileRange.createFileRange(1000, 100, "3") |
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.
Can we set one reference to null, such that null references gets tested as well.
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.
good idea. i will do that for range #2
Change-Id: I50cf741e76189a53a587de31cd69928536a986dd
|
🎊 +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.
+1 pending checkstyle issues.
Change-Id: I79b8be3ae8498fe02f85e47c270990293663f940
|
🎊 +1 overall
This message was automatically generated. |
|
in trunk; need to backport |
…5076) Contributed by Steve Loughran
…5076) Contributed by Steve Loughran
…pache#5076) Contributed by Steve Loughran
Add new reference field; verifies it remains in merging.
also fix up those tests which used string comparison of range values into actual field checking.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?