taklwu commented on code in PR #8851:
URL: https://github.com/apache/hbase/pull/8851#discussion_r4541083617
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java:
##########
@@ -361,24 +400,58 @@ ReaderContext createReaderContext(boolean doDropBehind,
long readahead, ReaderTy
newFnfe.initCause(fnfe);
throw newFnfe;
}
- status = fs.getFileStatus(referencePath);
- } else {
- in = new FSDataInputStreamWrapper(fs, this.getPath(), doDropBehind,
readahead);
- status = fs.getFileStatus(initialPath);
- }
- long length = status.getLen();
- ReaderContextBuilder contextBuilder =
- new
ReaderContextBuilder().withInputStreamWrapper(in).withFileSize(length)
-
.withPrimaryReplicaReader(this.primaryReplica).withReaderType(type).withFileSystem(fs)
-
.withSystemKeyCache(systemKeyCache).withManagedKeyDataCache(managedKeyDataCache);
- if (this.reference != null) {
- contextBuilder.withFilePath(this.getPath());
+ FileStatus refStatus = fs.getFileStatus(referencePath);
+ readerPath = this.getPath();
+ length = refStatus.getLen();
} else {
- contextBuilder.withFilePath(status.getPath());
- }
+ statusForOpen = getPlainHFileStatusForOpen();
+ in = new FSDataInputStreamWrapper(fs, initialPath, doDropBehind,
readahead, statusForOpen);
+ readerPath = statusForOpen.getPath();
+ length = size > 0 ? size : statusForOpen.getLen();
+ }
+ ReaderContextBuilder contextBuilder = new
ReaderContextBuilder().withInputStreamWrapper(in)
+
.withFileSize(length).withPrimaryReplicaReader(this.primaryReplica).withReaderType(type)
+ .withFileSystem(fs).withSystemKeyCache(systemKeyCache)
+ .withManagedKeyDataCache(managedKeyDataCache).withFilePath(readerPath);
return contextBuilder.build();
}
+ private boolean isPlainHFile() {
+ return this.reference == null && this.link == null;
+ }
+
+ /**
+ * Returns a {@link FileStatus} for plain HFiles, using a cached or
synthesized status when
+ * possible so reader open does not repeat metadata lookups.
+ */
+ private FileStatus getPlainHFileStatusForOpen() throws IOException {
+ // A complete HFile is never empty, so a zero-length cached status was
captured before the
+ // file was fully written and must be refreshed.
+ if (plainHFileStatus != null && plainHFileStatus.getLen() > 0) {
Review Comment:
They track different things. `plainHFileStatusNeedsRefresh` marks a status
after we built from a known size via `initPlainHFileStatusFromKnownSize()`
otherwise it should be considered as unset or false.
We found it during the failed `TestCachedMobFile`. It's valid for opening
the reader, but it doesn't have real block size or replication, so it's only
refreshed before computing block locality. The getLen() > 0 check catches a
real status captured before the file was written (e.g. TestCachedMobFile
creates the StoreFileInfo right after the writer opens the file). That status
has `plainHFileStatusNeedsRefresh` == false but length 0. Checking
`plainHFileStatusNeedsRefresh` here would re-stat every tracker-built status on
open, which removes the saving, and it would still reuse the zero-length one.
--
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]
