Skip to content

perf: Avoid re-fetching file status from FS for HFile readers #17709

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

Open
the-other-tim-brown wants to merge 4 commits into
base: master
Choose a base branch
from
+24 −12
Open

perf: Avoid re-fetching file status from FS for HFile readers #17709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d9ec00
avoid fetching file status from FS for hfile readers
the-other-tim-brown Dec 24, 2025
990eaf7
fix NPE
the-other-tim-brown Dec 24, 2025
efa0341
pass existing size to buffer
the-other-tim-brown Dec 24, 2025
14751d0
fix unit test
the-other-tim-brown Dec 24, 2025
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
24 changes: 18 additions & 6 deletions hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieHFileDataBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.apache.hudi.common.engine.HoodieReaderContext;
import org.apache.hudi.common.model.HoodieFileFormat;
import org.apache.hudi.common.model.HoodieLogFile;
import org.apache.hudi.common.model.HoodieRecord;
import org.apache.hudi.common.model.HoodieRecord.HoodieRecordType;
import org.apache.hudi.common.schema.HoodieSchema;
Expand All @@ -34,11 +35,10 @@
import org.apache.hudi.storage.HoodieStorage;
import org.apache.hudi.storage.StorageConfiguration;
import org.apache.hudi.storage.StoragePath;
import org.apache.hudi.storage.StoragePathInfo;
import org.apache.hudi.storage.inline.InLineFSUtils;

import org.apache.avro.generic.IndexedRecord;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand All @@ -57,8 +57,6 @@
* base file format.
*/
public class HoodieHFileDataBlock extends HoodieDataBlock {
private static final Logger LOG = LoggerFactory.getLogger(HoodieHFileDataBlock.class);

private final Option<String> compressionCodec;
// This path is used for constructing HFile reader context, which should not be
// interpreted as the actual file path for the HFile data blocks
Expand Down Expand Up @@ -164,12 +162,13 @@ protected <T> ClosableIterator<HoodieRecord<T>> lookupRecords(List<String> sorte
blockContentLoc.getLogFile().getPath().toUri().getScheme(),
blockContentLoc.getContentPositionInLogFile(),
blockContentLoc.getBlockSize());
StoragePathInfo storagePathInfo = getStoragePathInfo(inlinePath, blockContentLoc);
HoodieStorage inlineStorage = getBlockContentLocation().get().getStorage().newInstance(inlinePath, inlineConf);

try (final HoodieAvroHFileReaderImplBase reader = (HoodieAvroHFileReaderImplBase) HoodieIOFactory
.getIOFactory(inlineStorage)
.getReaderFactory(HoodieRecordType.AVRO)
.getFileReader(ConfigUtils.DEFAULT_HUDI_CONFIG_FOR_READER, inlinePath, HoodieFileFormat.HFILE, Option.of(getSchemaFromHeader()))) {
.getFileReader(ConfigUtils.DEFAULT_HUDI_CONFIG_FOR_READER, storagePathInfo, HoodieFileFormat.HFILE, Option.of(getSchemaFromHeader()))) {
// Get writer's schema from the header
final ClosableIterator<HoodieRecord<IndexedRecord>> recordIterator =
fullKey ? reader.getRecordsByKeysIterator(sortedKeys, readerSchema) :
Expand All @@ -190,18 +189,31 @@ protected <T> ClosableIterator<T> lookupEngineRecords(List<String> sortedKeys, b
blockContentLoc.getLogFile().getPath().toUri().getScheme(),
blockContentLoc.getContentPositionInLogFile(),
blockContentLoc.getBlockSize());
StoragePathInfo storagePathInfo = getStoragePathInfo(inlinePath, blockContentLoc);
HoodieStorage inlineStorage = blockContentLoc.getStorage().newInstance(inlinePath, inlineConf);

try (final HoodieAvroHFileReaderImplBase reader = (HoodieAvroHFileReaderImplBase) HoodieIOFactory
.getIOFactory(inlineStorage)
.getReaderFactory(HoodieRecordType.AVRO)
.getFileReader(ConfigUtils.DEFAULT_HUDI_CONFIG_FOR_READER,
inlinePath,
storagePathInfo,
HoodieFileFormat.HFILE,
Option.of(getSchemaFromHeader()))) {
// Get writer's schema from the header
return (ClosableIterator<T>) (fullKey ? reader.getEngineRecordsByKeysIterator(sortedKeys, readerSchema) :
reader.getEngineRecordsByKeyPrefixIterator(sortedKeys, readerSchema));
}
}

private static StoragePathInfo getStoragePathInfo(StoragePath inlinePath, HoodieLogBlockContentLocation blockContentLoc) {
HoodieLogFile logFile = blockContentLoc.getLogFile();
StoragePathInfo pathInfo = logFile.getPathInfo();
return new StoragePathInfo(
inlinePath,
blockContentLoc.getBlockSize(),
false,
pathInfo == null ? 0 : pathInfo.getBlockReplication(),
pathInfo == null ? 0L : pathInfo.getBlockSize(),
pathInfo == null ? 0L : pathInfo.getModificationTime());
}
}
2 changes: 1 addition & 1 deletion hudi-common/src/main/java/org/apache/hudi/io/storage/HFileReaderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private SeekableDataInputStream createInputStream(long fileSize) throws IOExcept
StoragePath path = fileSource.asLeft();
byte[] buffer;
try (SeekableDataInputStream stream = storage.openSeekable(path, false)) {
buffer = new byte[(int) storage.getPathInfo(path).getLength()];
buffer = new byte[(int) fileSize];
Copy link
Contributor

@yihua yihua Dec 24, 2025

Choose a reason for hiding this comment

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

For an inline path (inlinefs:), this represents the log block size instead of the full log file size. Will that be a problem?

Copy link
Contributor Author

@the-other-tim-brown the-other-tim-brown Dec 24, 2025

Choose a reason for hiding this comment

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

Is there a path that executes this that I can run locally?

Copy link
Contributor Author

@the-other-tim-brown the-other-tim-brown Jan 2, 2026

Choose a reason for hiding this comment

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

@yihua let me know if there is a particular path I can step through to validate this

stream.readFully(buffer);
}
return new ByteArraySeekableDataInputStream(new ByteBufferBackedInputStream(buffer));
Expand Down
2 changes: 1 addition & 1 deletion hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ private ClosableIterator<IndexedRecord> readSliceWithFilter(Predicate predicate,
if (fileSlice.getBaseFile().isPresent()) {
HoodieConfig fileGroupReaderConfig = new HoodieConfig(fileGroupReaderProps);
baseFileReader = (HoodieAvroFileReader) HoodieIOFactory.getIOFactory(getStorage()).getReaderFactory(HoodieRecord.HoodieRecordType.AVRO)
.getFileReader(fileGroupReaderConfig, fileSlice.getBaseFile().get().getStoragePath(), metadataMetaClient.getTableConfig().getBaseFileFormat(), Option.empty());
.getFileReader(fileGroupReaderConfig, fileSlice.getBaseFile().get().getPathInfo(), metadataMetaClient.getTableConfig().getBaseFileFormat(), Option.empty());
}
return Pair.of(baseFileReader, buildReusableRecordBufferLoader(fileSlice, latestMetadataInstantTime, instantRange));
} catch (IOException ex) {
Expand Down
2 changes: 1 addition & 1 deletion hudi-common/src/test/java/org/apache/hudi/io/storage/TestHFileReaderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void testCreateHFileReader_FileSizeBelowThreshold_ShouldUseContentCache() throws
assertInstanceOf(HFileReaderImpl.class, result);

// Verify that content was downloaded (cache was used)
verify(mockStorage, times(2)).getPathInfo(mockPath); // Once for size determination, once for download
verify(mockStorage, times(1)).getPathInfo(mockPath); // Once for size determination, which is reused for download
verify(mockStorage, times(1)).openSeekable(mockPath, false); // For content download
verify(mockInputStream, times(1)).readFully(any(byte[].class));
}
Expand Down
6 changes: 3 additions & 3 deletions ...p-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroFileReaderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ protected HoodieFileReader newHFileFileReader(HoodieConfig hoodieConfig,
HoodieStorage storage,
byte[] content,
Option<HoodieSchema> schemaOption) throws IOException {
HFileReaderFactory readerFactory = HFileReaderFactory.builder()
HFileReaderFactory.Builder readerFactoryBuilder = HFileReaderFactory.builder()
.withStorage(storage).withProps(hoodieConfig.getProps())
.withContent(content).build();
.withContent(content);
return HoodieNativeAvroHFileReader.builder()
.readerFactory(readerFactory).path(path).schema(schemaOption).build();
.readerFactory(readerFactoryBuilder.build()).path(path).schema(schemaOption).build();
}

@Override
Expand Down
Loading