nsivabalan commented on a change in pull request #4559:
URL: https://github.com/apache/hudi/pull/4559#discussion_r799842815
##########
File path:
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieFileInputFormatBase.java
##########
@@ -65,12 +67,32 @@
* <li>Incremental mode: reading table's state as of particular timestamp
(or instant, in Hudi's terms)</li>
* <li>External mode: reading non-Hudi partitions</li>
* </ul>
+ *
+ * NOTE: This class is invariant of the underlying file-format of the files
being read
*/
public abstract class HoodieFileInputFormatBase extends
FileInputFormat<NullWritable, ArrayWritable>
implements Configurable {
protected Configuration conf;
+ @Nonnull
+ private static RealtimeFileStatus
createRealtimeFileStatusUnchecked(HoodieBaseFile baseFile,
Stream<HoodieLogFile> logFiles) {
Review comment:
is this used anywhere? in like another stacked diff? I pulled this patch
down locally and don't see any usages for this method?
##########
File path:
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieFileInputFormatBase.java
##########
@@ -172,35 +198,25 @@ private static FileStatus
getFileStatusUnchecked(HoodieBaseFile baseFile) {
return HoodieInputFormatUtils.filterIncrementalFileStatus(jobContext,
tableMetaClient, timeline.get(), fileStatuses, commitsToCheck.get());
}
- protected abstract boolean includeLogFilesForSnapshotView();
+ @Override
+ protected FileSplit makeSplit(Path file, long start, long length,
+ String[] hosts, String[] inMemoryHosts) {
+ FileSplit split = new FileSplit(file, start, length, hosts, inMemoryHosts);
+ if (file instanceof PathWithBootstrapFileStatus) {
Review comment:
can you help me understand why don't I see diff FileStatuses here like
RealtimeFileStatus for eg, but just PathWithBootstrapFileStatus?
##########
File path:
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieFileInputFormatBase.java
##########
@@ -65,12 +67,32 @@
* <li>Incremental mode: reading table's state as of particular timestamp
(or instant, in Hudi's terms)</li>
* <li>External mode: reading non-Hudi partitions</li>
* </ul>
+ *
+ * NOTE: This class is invariant of the underlying file-format of the files
being read
*/
public abstract class HoodieFileInputFormatBase extends
FileInputFormat<NullWritable, ArrayWritable>
implements Configurable {
protected Configuration conf;
+ @Nonnull
+ private static RealtimeFileStatus
createRealtimeFileStatusUnchecked(HoodieBaseFile baseFile,
Stream<HoodieLogFile> logFiles) {
+ List<HoodieLogFile> sortedLogFiles =
logFiles.sorted(HoodieLogFile.getLogFileComparator()).collect(Collectors.toList());
+ FileStatus baseFileStatus = getFileStatusUnchecked(baseFile);
+ try {
+ RealtimeFileStatus rtFileStatus = new RealtimeFileStatus(baseFileStatus);
+ rtFileStatus.setDeltaLogFiles(sortedLogFiles);
+ rtFileStatus.setBaseFilePath(baseFile.getPath());
+ if (baseFileStatus instanceof LocatedFileStatusWithBootstrapBaseFile ||
baseFileStatus instanceof FileStatusWithBootstrapBaseFile) {
+ rtFileStatus.setBootStrapFileStatus(baseFileStatus);
Review comment:
does this mean, we are not going to have a separate fileStatus for
bootstrap and will be using RealTimeFileStatus for all ?
##########
File path:
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/realtime/HoodieHFileRealtimeInputFormat.java
##########
@@ -38,32 +36,18 @@
import org.apache.log4j.Logger;
import java.io.IOException;
-import java.util.Arrays;
-import java.util.List;
-import java.util.stream.Collectors;
/**
* HoodieRealtimeInputFormat for HUDI datasets which store data in HFile base
file format.
*/
@UseRecordReaderFromInputFormat
@UseFileSplitsFromInputFormat
-public class HoodieHFileRealtimeInputFormat extends HoodieHFileInputFormat {
+public class HoodieHFileRealtimeInputFormat extends
HoodieRealtimeFileInputFormatBase {
private static final Logger LOG =
LogManager.getLogger(HoodieHFileRealtimeInputFormat.class);
- @Override
- public InputSplit[] getSplits(JobConf job, int numSplits) throws IOException
{
- List<FileSplit> fileSplits = Arrays.stream(super.getSplits(job, numSplits))
- .map(is -> (FileSplit) is)
- .collect(Collectors.toList());
- return HoodieRealtimeInputFormatUtils.getRealtimeSplits(job, fileSplits);
- }
-
- @Override
- protected HoodieDefaultTimeline filterInstantsTimeline(HoodieDefaultTimeline
timeline) {
- // no specific filtering for Realtime format
- return timeline;
- }
+ // NOTE: We're only using {@code HoodieHFileInputFormat} to compose {@code
RecordReader}
+ private final HoodieHFileInputFormat hFileInputFormat = new
HoodieHFileInputFormat();
Review comment:
There are quite a few things different in HFileRealtimeInputFormat
compared to ParquetRealtimeInputFormat. for eg, getSplits just does
HoodieRealtimeInputFormatUtils.getRealtimeSplits(job, fileSplits) in case of
HFile, where as in case of Parquet, we do both realtimeSplits and
incrementalSplits.
and then
```
@Override
protected boolean includeLogFilesForSnapshotView() {
return false;
}
```
```
@Override
protected boolean isSplitable(FileSystem fs, Path filename) {
// This file isn't splittable.
return false;
}
```
these are diff in parquet real time IF.
filterInstantsTimeline(HoodieDefaultTimeline timeline) is empty in case of
HFileRTIF. after unification, I don't see these are overridden here in
HoodieHFileRealtimeInputFormat.
can you help me understand please.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]