diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index 22e3901e2240..48375287bcd9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -1291,8 +1291,7 @@ public static HDFSBlocksDistribution computeHDFSBlocksDistribution(Configuration
// Only construct StoreFileInfo object if its not a hfile, save obj
// creation
StoreFileInfo storeFileInfo = new StoreFileInfo(conf, fs, status);
- hdfsBlocksDistribution.add(storeFileInfo
- .computeHDFSBlocksDistribution(fs));
+ hdfsBlocksDistribution.add(storeFileInfo.computeHDFSBlocksDistribution());
} else if (StoreFileInfo.isHFile(p)) {
// If its a HFile, then lets just add to the block distribution
// lets not create more objects here, not even another HDFSBlocksDistribution
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
index 5eaab23fc6cf..346a2429170f 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
@@ -1,5 +1,4 @@
-/**
- *
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -18,13 +17,12 @@
*/
package org.apache.hadoop.hbase.regionserver;
-
import java.io.FileNotFoundException;
import java.io.IOException;
+import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
@@ -46,7 +44,8 @@
import org.slf4j.LoggerFactory;
/**
- * Describe a StoreFile (hfile, reference, link)
+ * StoreFile info.
+ * The info could be for a plain storefile/hfile or it could be a reference or link to a storefile.
*/
@InterfaceAudience.Private
public class StoreFileInfo {
@@ -111,6 +110,16 @@ public class StoreFileInfo {
// done.
final AtomicInteger refCount = new AtomicInteger(0);
+ /**
+ * Cached fileStatus.
+ * Used selectively. Cache the FileStatus if this StoreFileInfo is for a plain StoreFile.
+ * Save on having to go to the filesystem every time (costly). We cannot cache FileStatus if this
+ * StoreFileInfo is for a link or for a reference that might in turn be to a link. The file behind
+ * a link can move during the lifetime of this StoreFileInfo invalidating what we might have
+ * cached here; do a lookup of FileStatus every time when a link to be safe.
+ */
+ private FileStatus cachedFileStatus = null;
+
/**
* Create a Store File Info
* @param conf the {@link Configuration} to use
@@ -125,47 +134,38 @@ public StoreFileInfo(final Configuration conf, final FileSystem fs, final Path i
private StoreFileInfo(final Configuration conf, final FileSystem fs, final FileStatus fileStatus,
final Path initialPath, final boolean primaryReplica) throws IOException {
- assert fs != null;
- assert initialPath != null;
- assert conf != null;
-
- this.fs = fs;
- this.conf = conf;
- this.initialPath = initialPath;
+ this.fs = Objects.requireNonNull(fs);
+ this.conf = Objects.requireNonNull(conf);
+ this.initialPath = Objects.requireNonNull(initialPath);
this.primaryReplica = primaryReplica;
this.noReadahead = this.conf.getBoolean(STORE_FILE_READER_NO_READAHEAD,
DEFAULT_STORE_FILE_READER_NO_READAHEAD);
- Path p = initialPath;
- if (HFileLink.isHFileLink(p)) {
- // HFileLink
+ if (HFileLink.isHFileLink(initialPath)) {
this.reference = null;
- this.link = HFileLink.buildFromHFileLinkPattern(conf, p);
- LOG.trace("{} is a link", p);
- } else if (isReference(p)) {
- this.reference = Reference.read(fs, p);
- Path referencePath = getReferredToFile(p);
- if (HFileLink.isHFileLink(referencePath)) {
- // HFileLink Reference
- this.link = HFileLink.buildFromHFileLinkPattern(conf, referencePath);
- } else {
- // Reference
- this.link = null;
- }
- LOG.trace("{} is a {} reference to {}", p, reference.getFileRegion(), referencePath);
- } else if (isHFile(p) || isMobFile(p) || isMobRefFile(p)) {
- // HFile
- if (fileStatus != null) {
- this.createdTimestamp = fileStatus.getModificationTime();
- this.size = fileStatus.getLen();
- } else {
- FileStatus fStatus = fs.getFileStatus(initialPath);
- this.createdTimestamp = fStatus.getModificationTime();
- this.size = fStatus.getLen();
- }
+ this.link = HFileLink.buildFromHFileLinkPattern(conf, initialPath);
+ LOG.trace("{} is a link", initialPath);
+ } else if (isReference(initialPath)) {
+ this.reference = Reference.read(fs, initialPath);
+ Path referencedPath = getReferredToFile(initialPath);
+ // Check if the referenced file is a link.
+ this.link = HFileLink.isHFileLink(referencedPath)?
+ HFileLink.buildFromHFileLinkPattern(conf, referencedPath): null;
+ LOG.trace("{} is a {} reference to {} (link={})",
+ initialPath, reference.getFileRegion(), referencedPath, link == null);
+ } else if (isHFile(initialPath) || isMobFile(initialPath) || isMobRefFile(initialPath)) {
+ // Safe to cache passed filestatus when NOT a link or reference; i.e. when it filestatus on
+ // a plain storefile/hfile.
+ assert fileStatus == null || fileStatus.getPath().equals(initialPath);
+ this.cachedFileStatus = fileStatus != null?
+ fileStatus: this.fs.getFileStatus(this.initialPath);
+ this.createdTimestamp = this.cachedFileStatus.getModificationTime();
+ this.size = this.cachedFileStatus.getLen();
this.reference = null;
this.link = null;
+ LOG.trace("{}", initialPath);
} else {
- throw new IOException("path=" + p + " doesn't look like a valid StoreFile");
+ throw new IOException("Path=" + initialPath + " doesn't look like a valid StoreFile; " +
+ "it is not a link, a reference or an hfile.");
}
}
@@ -213,8 +213,8 @@ public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileSt
*/
public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileStatus fileStatus,
final Reference reference, final HFileLink link) {
- this.fs = fs;
- this.conf = conf;
+ this.fs = Objects.requireNonNull(fs);
+ this.conf = Objects.requireNonNull(conf);
this.primaryReplica = false;
this.initialPath = (fileStatus == null) ? null : fileStatus.getPath();
this.createdTimestamp = (fileStatus == null) ? 0 : fileStatus.getModificationTime();
@@ -224,6 +224,17 @@ public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileSt
DEFAULT_STORE_FILE_READER_NO_READAHEAD);
}
+ public StoreFileInfo(StoreFileInfo other, FileSystem fileSystem) {
+ this.fs = fileSystem;
+ this.conf = other.conf;
+ this.primaryReplica = other.primaryReplica;
+ this.initialPath = other.initialPath;
+ this.createdTimestamp = other.createdTimestamp;
+ this.link = other.link;
+ this.reference = other.reference;
+ this.noReadahead = other.noReadahead;
+ }
+
/**
* Size of the Hfile
* @return size
@@ -234,7 +245,6 @@ public long getSize() {
/**
* Sets the region coprocessor env.
- * @param coprocessorHost
*/
public void setRegionCoprocessorHost(RegionCoprocessorHost coprocessorHost) {
this.coprocessorHost = coprocessorHost;
@@ -270,13 +280,9 @@ public HDFSBlocksDistribution getHDFSBlockDistribution() {
}
StoreFileReader createReader(ReaderContext context, CacheConfig cacheConf) throws IOException {
- StoreFileReader reader = null;
- if (this.reference != null) {
- reader = new HalfStoreFileReader(context, hfileInfo, cacheConf, reference, refCount, conf);
- } else {
- reader = new StoreFileReader(context, hfileInfo, cacheConf, refCount, conf);
- }
- return reader;
+ return this.reference != null?
+ new HalfStoreFileReader(context, hfileInfo, cacheConf, reference, refCount, conf):
+ new StoreFileReader(context, hfileInfo, cacheConf, refCount, conf);
}
ReaderContext createReaderContext(boolean doDropBehind, long readahead, ReaderType type)
@@ -293,22 +299,21 @@ ReaderContext createReaderContext(boolean doDropBehind, long readahead, ReaderTy
try {
in = new FSDataInputStreamWrapper(fs, referencePath, doDropBehind, readahead);
} catch (FileNotFoundException fnfe) {
- // Intercept the exception so can insert more info about the Reference; otherwise
- // exception just complains about some random file -- operator doesn't realize it
- // other end of a Reference
- FileNotFoundException newFnfe = new FileNotFoundException(toString());
- newFnfe.initCause(fnfe);
- throw newFnfe;
+ throw decorateFileNotFoundException(fnfe);
}
status = fs.getFileStatus(referencePath);
} else {
in = new FSDataInputStreamWrapper(fs, this.getPath(), doDropBehind, readahead);
- status = fs.getFileStatus(initialPath);
+ if (this.cachedFileStatus == null) {
+ // Safe to cache filestatus for a plain storefile/hfile.
+ this.cachedFileStatus = this.fs.getFileStatus(this.initialPath);
+ }
+ status = this.cachedFileStatus;
}
long length = status.getLen();
- ReaderContextBuilder contextBuilder =
- new ReaderContextBuilder().withInputStreamWrapper(in).withFileSize(length)
- .withPrimaryReplicaReader(this.primaryReplica).withReaderType(type).withFileSystem(fs);
+ ReaderContextBuilder contextBuilder = new ReaderContextBuilder().withInputStreamWrapper(in).
+ withFileSize(length).withPrimaryReplicaReader(this.primaryReplica).withReaderType(type).
+ withFileSystem(fs);
if (this.reference != null) {
contextBuilder.withFilePath(this.getPath());
} else {
@@ -320,76 +325,70 @@ ReaderContext createReaderContext(boolean doDropBehind, long readahead, ReaderTy
/**
* Compute the HDFS Block Distribution for this StoreFile
*/
- public HDFSBlocksDistribution computeHDFSBlocksDistribution(final FileSystem fs)
- throws IOException {
- // guard against the case where we get the FileStatus from link, but by the time we
- // call compute the file is moved again
+ public HDFSBlocksDistribution computeHDFSBlocksDistribution() throws IOException {
if (this.link != null) {
+ // Guard against case where file behind link has moved when we go to calculate distribution;
+ // e.g. from data dir to archive dir. Retry up to number of locations under the link.
FileNotFoundException exToThrow = null;
for (int i = 0; i < this.link.getLocations().length; i++) {
try {
- return computeHDFSBlocksDistributionInternal(fs);
- } catch (FileNotFoundException ex) {
- // try the other location
- exToThrow = ex;
+ return computeHDFSBlocksDistributionInternal();
+ } catch (FileNotFoundException fnfe) {
+ // Try the other locations -- file behind link may have moved.
+ exToThrow = fnfe;
}
}
- throw exToThrow;
+ throw decorateFileNotFoundException(exToThrow);
} else {
- return computeHDFSBlocksDistributionInternal(fs);
+ return computeHDFSBlocksDistributionInternal();
}
}
- private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileSystem fs)
- throws IOException {
- FileStatus status = getReferencedFileStatus(fs);
+ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal() throws IOException {
+ FileStatus status = getFileStatus();
if (this.reference != null) {
- return computeRefFileHDFSBlockDistribution(fs, reference, status);
+ return computeRefFileHDFSBlockDistribution(status);
} else {
- return FSUtils.computeHDFSBlocksDistribution(fs, status, 0, status.getLen());
+ return FSUtils.computeHDFSBlocksDistribution(this.fs, status, 0, status.getLen());
}
}
/**
- * Get the {@link FileStatus} of the file referenced by this StoreFileInfo
- * @param fs The current file system to use.
+ * Get the {@link FileStatus} of the file referenced by this StoreFileInfo.
+ * This {@link StoreFileInfo} could be for a link or a reference or a plain hfile/storefile; get
+ * the filestatus for whatever the link or reference points to (or just the plain hfile/storefile
+ * if not a link/reference). Info}. If a link, when you go to use the passed FileStatus, the file
+ * may have moved; e.g. from data to archive... be aware.
* @return The {@link FileStatus} of the file referenced by this StoreFileInfo
*/
- public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOException {
+ private FileStatus getReferencedFileStatus() throws IOException {
+ if (this.cachedFileStatus != null) {
+ return this.cachedFileStatus;
+ }
FileStatus status;
if (this.reference != null) {
if (this.link != null) {
- FileNotFoundException exToThrow = null;
- for (int i = 0; i < this.link.getLocations().length; i++) {
- // HFileLink Reference
- try {
- return link.getFileStatus(fs);
- } catch (FileNotFoundException ex) {
- // try the other location
- exToThrow = ex;
- }
- }
- throw exToThrow;
+ status = this.link.getFileStatus(this.fs);
} else {
- // HFile Reference
- Path referencePath = getReferredToFile(this.getPath());
- status = fs.getFileStatus(referencePath);
+ try {
+ Path referencePath = getReferredToFile(this.getPath());
+ status = this.fs.getFileStatus(referencePath);
+ } catch (FileNotFoundException ex) {
+ throw decorateFileNotFoundException(ex);
+ }
}
} else {
- if (this.link != null) {
- FileNotFoundException exToThrow = null;
- for (int i = 0; i < this.link.getLocations().length; i++) {
- // HFileLink
- try {
- return link.getFileStatus(fs);
- } catch (FileNotFoundException ex) {
- // try the other location
- exToThrow = ex;
- }
+ try {
+ if (this.link != null) {
+ status = this.link.getFileStatus(this.fs);
+ } else {
+ status = this.fs.getFileStatus(this.initialPath);
+ // Take this opportunity to cache the filestatus. It is safe to cache filestatus when NOT
+ // a link or reference; i.e. when it filestatus on a plain storefile/hfile.
+ this.cachedFileStatus = status;
}
- throw exToThrow;
- } else {
- status = fs.getFileStatus(initialPath);
+ } catch (FileNotFoundException fnfe) {
+ throw decorateFileNotFoundException(fnfe);
}
}
return status;
@@ -400,9 +399,22 @@ public Path getPath() {
return initialPath;
}
- /** @return The {@link FileStatus} of the file */
+ /**
+ * @return A new FNFE with fnfe as cause but including info if reference or link.
+ */
+ private FileNotFoundException decorateFileNotFoundException(FileNotFoundException fnfe) {
+ FileNotFoundException newFnfe = new FileNotFoundException(toString());
+ newFnfe.initCause(fnfe);
+ return newFnfe;
+ }
+
+ /**
+ * @return {@link FileStatus} for the linked or referenced file or if not a link/reference, then
+ * the FileStatus for the plain storefile (Be aware, if a link, the file may have moved
+ * by the time you go to use the FileStatus).
+ */
public FileStatus getFileStatus() throws IOException {
- return getReferencedFileStatus(fs);
+ return getReferencedFileStatus();
}
/** @return Get the modification time of the file. */
@@ -412,8 +424,8 @@ public long getModificationTime() throws IOException {
@Override
public String toString() {
- return this.getPath()
- + (isReference() ? "->" + getReferredToFile(this.getPath()) + "-" + reference : "");
+ return isLink()? this.link.toString(): this.getPath() +
+ (isReference()? "->" + getReferredToFile(this.getPath()) + "-" + reference: "");
}
/**
@@ -523,10 +535,7 @@ public static Path getReferredToFile(final Path p) {
* @return true if the file could be a valid store file, false otherwise
*/
public static boolean validateStoreFileName(final String fileName) {
- if (HFileLink.isHFileLink(fileName) || isReference(fileName)) {
- return true;
- }
- return !fileName.contains("-");
+ return (HFileLink.isHFileLink(fileName) || isReference(fileName)) || !fileName.contains("-");
}
/**
@@ -553,29 +562,27 @@ public static boolean isValid(final FileStatus fileStatus) throws IOException {
}
/**
- * helper function to compute HDFS blocks distribution of a given reference file.For reference
- * file, we don't compute the exact value. We use some estimate instead given it might be good
- * enough. we assume bottom part takes the first half of reference file, top part takes the second
+ * Helper function to compute HDFS blocks distribution of a given reference file. For reference
+ * file, we don't compute the exact value. We use an estimate instead presuming it good enough.
+ * We assume bottom part takes the first half of a reference file, top part takes the second
* half of the reference file. This is just estimate, given midkey ofregion != midkey of HFile,
* also the number and size of keys vary. If this estimate isn't good enough, we can improve it
* later.
- * @param fs The FileSystem
- * @param reference The reference
- * @param status The reference FileStatus
+ * @param status The reference FileStatus
* @return HDFS blocks distribution
*/
- private static HDFSBlocksDistribution computeRefFileHDFSBlockDistribution(final FileSystem fs,
- final Reference reference, final FileStatus status) throws IOException {
+ private HDFSBlocksDistribution computeRefFileHDFSBlockDistribution(final FileStatus status)
+ throws IOException {
if (status == null) {
return null;
}
- long start = 0;
- long length = 0;
+ long start;
+ long length;
- if (Reference.isTopFileRegion(reference.getFileRegion())) {
+ if (Reference.isTopFileRegion(this.reference.getFileRegion())) {
start = status.getLen() / 2;
- length = status.getLen() - status.getLen() / 2;
+ length = status.getLen() - (status.getLen() / 2);
} else {
start = 0;
length = status.getLen() / 2;
@@ -596,15 +603,15 @@ public boolean equals(Object that) {
return false;
}
- StoreFileInfo o = (StoreFileInfo) that;
+ StoreFileInfo o = (StoreFileInfo)that;
if (initialPath != null && o.initialPath == null) {
return false;
}
if (initialPath == null && o.initialPath != null) {
return false;
}
- if (initialPath != o.initialPath && initialPath != null
- && !initialPath.equals(o.initialPath)) {
+ if (initialPath != o.initialPath && initialPath != null &&
+ !initialPath.equals(o.initialPath)) {
return false;
}
if (reference != null && o.reference == null) {
@@ -613,8 +620,8 @@ public boolean equals(Object that) {
if (reference == null && o.reference != null) {
return false;
}
- if (reference != o.reference && reference != null
- && !reference.equals(o.reference)) {
+ if (reference != o.reference && reference != null &&
+ !reference.equals(o.reference)) {
return false;
}
@@ -627,7 +634,6 @@ public boolean equals(Object that) {
if (link != o.link && link != null && !link.equals(o.link)) {
return false;
}
-
return true;
}
@@ -659,6 +665,13 @@ FileSystem getFileSystem() {
return this.fs;
}
+ /**
+ * @return True if passed filesystem is same as the one this instance was created against.
+ */
+ public boolean isFileSystem(FileSystem other) {
+ return this.fs.equals(other);
+ }
+
Configuration getConf() {
return this.conf;
}
@@ -672,7 +685,7 @@ HFileInfo getHFileInfo() {
}
void initHDFSBlocksDistribution() throws IOException {
- hdfsBlocksDistribution = computeHDFSBlocksDistribution(fs);
+ hdfsBlocksDistribution = computeHDFSBlocksDistribution();
}
StoreFileReader preStoreFileReaderOpen(ReaderContext context, CacheConfig cacheConf)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifestV2.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifestV2.java
index ae914f69b5cc..4421fc44cb5f 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifestV2.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifestV2.java
@@ -130,7 +130,10 @@ public void storeFile(final SnapshotRegionManifest.Builder region,
if (!storeFile.isReference() && !storeFile.isLink()) {
sfManifest.setFileSize(storeFile.getSize());
} else {
- sfManifest.setFileSize(storeFile.getReferencedFileStatus(rootFs).getLen());
+ long len = storeFile.isFileSystem(this.rootFs)?
+ storeFile.getFileStatus().getLen():
+ (new StoreFileInfo(storeFile, this.rootFs)).getFileStatus().getLen();
+ sfManifest.setFileSize(len);
}
family.addStoreFiles(sfManifest.build());
}