Skip to content

HADOOP-18321.Fix when to read an additional record from a BZip2 text file split #4521

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.

Sign up for GitHub

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

Merged
PrabhuJoseph merged 3 commits into from
Jul 6, 2022
Merged

HADOOP-18321.Fix when to read an additional record from a BZip2 text file split #4521

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9261ada
HADOOP-18321.Fix when to read an additional record from a BZip2 text …
Jun 30, 2022
7a6efbb
Fixed Stylecheck
Jun 30, 2022
a0f755e
removing extra method
Jul 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/LineRecordReaderHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public long countRecords(long start, long length) throws IOException {

private LineRecordReader newReader(long start, long length)
throws IOException {
FileSplit split = new FileSplit(filePath, start, length, (String[]) null);
return new LineRecordReader(conf, split, recordDelimiterBytes);
FileSplit split = new FileSplit(getFilePath(), start, length, (String[]) null);
return new LineRecordReader(getConf(), split, getRecordDelimiterBytes());
}
}
2 changes: 1 addition & 1 deletion ...preduce-client-core/src/test/java/org/apache/hadoop/mapred/TestLineRecordReaderBZip2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ public final class TestLineRecordReaderBZip2 extends

@Override
protected BaseLineRecordReaderHelper newReader(Path file) {
return new LineRecordReaderHelper(file, conf);
return new LineRecordReaderHelper(file, getConf());
}
}
22 changes: 18 additions & 4 deletions ...-core/src/test/java/org/apache/hadoop/mapreduce/lib/input/BaseLineRecordReaderHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@

public abstract class BaseLineRecordReaderHelper {

protected final Configuration conf;
protected final Path filePath;
protected final byte[] recordDelimiterBytes;
private final Configuration conf;
private final Path filePath;
private final byte[] recordDelimiterBytes;



public BaseLineRecordReaderHelper(Path filePath, Configuration conf) {
this.conf = conf;
Expand All @@ -36,9 +38,21 @@ public BaseLineRecordReaderHelper(Path filePath, Configuration conf) {
conf.setInt(LineRecordReader.MAX_LINE_LENGTH, Integer.MAX_VALUE);

String delimiter = conf.get("textinputformat.record.delimiter");
recordDelimiterBytes =
this.recordDelimiterBytes =
null != delimiter ? delimiter.getBytes(StandardCharsets.UTF_8) : null;
}

public abstract long countRecords(long start, long length) throws IOException;

public Configuration getConf() {
return conf;
}

public Path getFilePath() {
return filePath;
}

public byte[] getRecordDelimiterBytes() {
return recordDelimiterBytes;
}
}
32 changes: 22 additions & 10 deletions ...re/src/test/java/org/apache/hadoop/mapreduce/lib/input/BaseTestLineRecordReaderBZip2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,21 @@ public abstract class BaseTestLineRecordReaderBZip2 {
private static final byte[] CR = new byte[] {'\r'};
private static final byte[] CR_LF = new byte[] {'\r', '\n'};

protected Configuration conf;
protected FileSystem fs;
protected Path tempFile;
private Configuration conf;
private FileSystem fs;
private Path tempFile;

public Configuration getConf() {
return conf;
}

public FileSystem getFs() {
return fs;
}

public Path getTempFile() {
return tempFile;
}

@Before
public void setUp() throws Exception {
Expand Down Expand Up @@ -78,7 +90,7 @@ public void firstBlockEndsWithLF() throws Exception {
}

@Test
public void firstBlockEndsWithLF_secondBlockStartsWithLF() throws Exception {
public void firstBlockEndsWithLFSecondBlockStartsWithLF() throws Exception {
try (BZip2TextFileWriter writer = new BZip2TextFileWriter(tempFile, conf)) {
writer.writeManyRecords(BLOCK_SIZE, 1000, LF);
// Write 254 empty rows terminating at LF, as those records will get
Expand All @@ -100,7 +112,7 @@ public void firstBlockEndsWithLF_secondBlockStartsWithLF() throws Exception {
}

@Test
public void firstBlockEndsWithLF_secondBlockStartsWithCR() throws Exception {
public void firstBlockEndsWithLFSecondBlockStartsWithCR() throws Exception {
try (BZip2TextFileWriter writer = new BZip2TextFileWriter(tempFile, conf)) {
writer.writeManyRecords(BLOCK_SIZE, 1000, LF);
writer.writeRecord(1, CR);
Expand Down Expand Up @@ -176,7 +188,7 @@ public void lastRecordOfFirstBlockHasItsCRLFPartlyInSecondBlock()
}

@Test
public void lastByteInFirstBlockIsCR_FirstByteInSecondBlockIsNotLF()
public void lastByteInFirstBlockIsCRFirstByteInSecondBlockIsNotLF()
throws Exception {
try (BZip2TextFileWriter writer = new BZip2TextFileWriter(tempFile, conf)) {
writer.writeManyRecords(BLOCK_SIZE, 1000, CR);
Expand Down Expand Up @@ -214,7 +226,7 @@ public void delimitedByCRSpanningThreeBlocks() throws Exception {
}

@Test
public void customDelimiter_lastThreeBytesInBlockAreDelimiter()
public void customDelimiterLastThreeBytesInBlockAreDelimiter()
throws Exception {
byte[] delimiter = new byte[] {'e', 'n', 'd'};
setDelimiter(delimiter);
Expand All @@ -229,7 +241,7 @@ public void customDelimiter_lastThreeBytesInBlockAreDelimiter()
}

@Test
public void customDelimiter_delimiterSpansAcrossBlocks()
public void customDelimiterDelimiterSpansAcrossBlocks()
throws Exception {
byte[] delimiter = new byte[] {'e', 'n', 'd'};
setDelimiter(delimiter);
Expand All @@ -245,7 +257,7 @@ public void customDelimiter_delimiterSpansAcrossBlocks()
}

@Test
public void customDelimiter_lastRecordDelimiterStartsAtNextBlockStart()
public void customDelimiterLastRecordDelimiterStartsAtNextBlockStart()
throws Exception {
byte[] delimiter = new byte[] {'e', 'n', 'd'};
setDelimiter(delimiter);
Expand All @@ -261,7 +273,7 @@ public void customDelimiter_lastRecordDelimiterStartsAtNextBlockStart()
}

@Test
public void customDelimiter_lastBlockBytesShareCommonPrefixWithDelimiter()
public void customDelimiterLastBlockBytesShareCommonPrefixWithDelimiter()
throws Exception {
byte[] delimiter = new byte[] {'e', 'n', 'd'};
setDelimiter(delimiter);
Expand Down
6 changes: 3 additions & 3 deletions ...ient-core/src/test/java/org/apache/hadoop/mapreduce/lib/input/LineRecordReaderHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ public long countRecords(long start, long length) throws IOException {

private LineRecordReader newReader(long start, long length)
throws IOException {
FileSplit split = new FileSplit(filePath, start, length, null);
FileSplit split = new FileSplit(getFilePath(), start, length, null);

TaskAttemptContext context =
new TaskAttemptContextImpl(conf, new TaskAttemptID());
new TaskAttemptContextImpl(getConf(), new TaskAttemptID());

LineRecordReader reader = new LineRecordReader(recordDelimiterBytes);
LineRecordReader reader = new LineRecordReader(getRecordDelimiterBytes());
try {
reader.initialize(split, context);
return reader;
Expand Down
7 changes: 6 additions & 1 deletion ...t-core/src/test/java/org/apache/hadoop/mapreduce/lib/input/TestLineRecordReaderBZip2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@
public final class TestLineRecordReaderBZip2
extends BaseTestLineRecordReaderBZip2 {

@Override
public void setUp() throws Exception {
super.setUp();
}
Copy link
Member

@aajisaka aajisaka Jul 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the lines are not required for the checkstyle fix.

Copy link
Contributor Author

@hotcodemacha hotcodemacha Jul 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @aajisaka - I have addressed the above comment.


@Override
protected BaseLineRecordReaderHelper newReader(Path file) {
return new LineRecordReaderHelper(file, conf);
return new LineRecordReaderHelper(file, getConf());
}
}