[
https://issues.apache.org/jira/browse/HADOOP-18507?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17624030#comment-17624030
]
ASF GitHub Bot commented on HADOOP-18507:
-----------------------------------------
mukund-thakur commented on code in PR #5076:
URL: https://github.com/apache/hadoop/pull/5076#discussion_r1004918931
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileRange.java:
##########
@@ -55,13 +55,33 @@ public interface FileRange {
*/
void setData(CompletableFuture<ByteBuffer> data);
+ /**
+ * 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
Review Comment:
typo
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestVectoredReadUtils.java:
##########
@@ -96,52 +96,59 @@ public void testRounding() {
@Test
public void testMerge() {
- FileRange base = FileRange.createFileRange(2000, 1000);
+ // a reference to use for tracking
+ Object tracker1 = "one";
+ Object tracker2 = "two";
+ FileRange base = FileRange.createFileRange(2000, 1000, tracker1);
CombinedFileRange mergeBase = new CombinedFileRange(2000, 3000, base);
// test when the gap between is too big
assertFalse("Large gap ranges shouldn't get merged", mergeBase.merge(5000,
6000,
FileRange.createFileRange(5000, 1000), 2000, 4000));
assertEquals("Number of ranges in merged range shouldn't increase",
1, mergeBase.getUnderlying().size());
- assertEquals("post merge offset", 2000, mergeBase.getOffset());
- assertEquals("post merge length", 1000, mergeBase.getLength());
+ assertFileRange(mergeBase, 2000, 1000);
// test when the total size gets exceeded
assertFalse("Large size ranges shouldn't get merged",
mergeBase.merge(5000, 6000,
FileRange.createFileRange(5000, 1000), 2001, 3999));
assertEquals("Number of ranges in merged range shouldn't increase",
1, mergeBase.getUnderlying().size());
- assertEquals("post merge offset", 2000, mergeBase.getOffset());
- assertEquals("post merge length", 1000, mergeBase.getLength());
+ assertFileRange(mergeBase, 2000, 1000);
// test when the merge works
assertTrue("ranges should get merged ", mergeBase.merge(5000, 6000,
- FileRange.createFileRange(5000, 1000), 2001, 4000));
+ FileRange.createFileRange(5000, 1000, tracker2),
+ 2001, 4000));
assertEquals("post merge size", 2, mergeBase.getUnderlying().size());
- assertEquals("post merge offset", 2000, mergeBase.getOffset());
- assertEquals("post merge length", 4000, mergeBase.getLength());
+ assertFileRange(mergeBase, 2000, 4000);
+
+ Assertions.assertThat(mergeBase.getUnderlying().get(0).getReference())
+ .describedAs("reference of range %s", mergeBase.getUnderlying().get(0))
+ .isSameAs(tracker1);
+ Assertions.assertThat(mergeBase.getUnderlying().get(1).getReference())
+ .describedAs("reference of range %s", mergeBase.getUnderlying().get(1))
+ .isSameAs(tracker2);
// reset the mergeBase and test with a 10:1 reduction
mergeBase = new CombinedFileRange(200, 300, base);
- assertEquals(200, mergeBase.getOffset());
- assertEquals(100, mergeBase.getLength());
+ assertFileRange(mergeBase, 200, 100);
+
assertTrue("ranges should get merged ", mergeBase.merge(500, 600,
FileRange.createFileRange(5000, 1000), 201, 400));
assertEquals("post merge size", 2, mergeBase.getUnderlying().size());
- assertEquals("post merge offset", 200, mergeBase.getOffset());
- assertEquals("post merge length", 400, mergeBase.getLength());
+ assertFileRange(mergeBase, 200, 400);
}
@Test
public void testSortAndMerge() {
List<FileRange> input = Arrays.asList(
- FileRange.createFileRange(3000, 100),
- FileRange.createFileRange(2100, 100),
- FileRange.createFileRange(1000, 100)
+ FileRange.createFileRange(3000, 100, "1"),
+ FileRange.createFileRange(2100, 100, "2"),
+ FileRange.createFileRange(1000, 100, "3")
Review Comment:
Can we set one reference to null, such that null references gets tested as
well.
> VectorIO FileRange type to support a "reference" field
> ------------------------------------------------------
>
> Key: HADOOP-18507
> URL: https://issues.apache.org/jira/browse/HADOOP-18507
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs
> Affects Versions: 3.3.5
> Reporter: Steve Loughran
> Assignee: Steve Loughran
> Priority: Major
> Labels: pull-request-available
>
> to use in libraries, it is really good to be able to connect a FileRange back
> to the application/library level structure (chunk/split data, usually).
> Proposed: add an {{Object reference)) field which can be given arbitrary data
> or null, and queried for by app. it is not used in the API at all
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
