Skip to content

HBASE-23584 : Descrease rpc getFileStatus count when open a storefile #1380

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

Closed
HuiHang-Yu wants to merge 1 commit into from
Closed

HBASE-23584 : Descrease rpc getFileStatus count when open a storefile #1380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 58 additions & 49 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
public class StoreFileInfo {
private static final Logger LOG = LoggerFactory.getLogger(StoreFileInfo.class);

private FileStatus localStatus = null;

/**
* A non-capture group, for hfiles, so that this can be embedded. HFiles are uuid ([0-9a-z]+).
* Bulk loaded hfiles has (_SeqId_[0-9]+_) has suffix. The mob del file has (_del) as suffix.
Expand Down Expand Up @@ -155,20 +157,61 @@ private StoreFileInfo(final Configuration conf, final FileSystem fs, final FileS
} else if (isHFile(p) || isMobFile(p) || isMobRefFile(p)) {
// HFile
if (fileStatus != null) {
this.createdTimestamp = fileStatus.getModificationTime();
this.size = fileStatus.getLen();
this.localStatus = fileStatus;
} else {
FileStatus fStatus = fs.getFileStatus(initialPath);
this.createdTimestamp = fStatus.getModificationTime();
this.size = fStatus.getLen();
this.localStatus = loadAndCacheFileStatus(fs);
}
this.createdTimestamp = localStatus.getModificationTime();
this.size = localStatus.getLen();
this.reference = null;
this.link = null;
} else {
throw new IOException("path=" + p + " doesn't look like a valid StoreFile");
}
}

private FileStatus loadAndCacheFileStatus(final FileSystem fs) throws IOException {
FileStatus status = null;
if (this.reference != null) {
if (this.link != null) {
for (int i = 0; i < this.link.getLocations().length; i++) {
// HFileLink Reference
try {
status = link.getFileStatus(fs);
} catch (FileNotFoundException ex) {
// try the other location
throw ex;
Copy link
Contributor

@binlijin binlijin Apr 29, 2020

Choose a reason for hiding this comment

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

just throw ex it will have no chance to try the other location, see the old code, it should keep the behave with old code.

}
}
} else {
try{
// HFile Reference
Path referencePath = getReferredToFile(this.getPath());
status = fs.getFileStatus(referencePath);
} catch (FileNotFoundException ex) {
FileNotFoundException newFnfe = new FileNotFoundException(toString());
newFnfe.initCause(ex);
throw newFnfe;
}
}
} else {
if (this.link != null) {
for (int i = 0; i < this.link.getLocations().length; i++) {
// HFileLink
try {
status = link.getFileStatus(fs);
} catch (FileNotFoundException ex) {
// try the other location
throw ex;
}
}
} else {
status = fs.getFileStatus(initialPath);
}
}
return status;
}

/**
* Create a Store File Info
* @param conf the {@link Configuration} to use
Expand All @@ -187,7 +230,7 @@ public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileSt
* @param fileStatus The {@link FileStatus} of the file
*/
public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileStatus fileStatus,
final HFileLink link) {
final HFileLink link) throws IOException {
this(conf, fs, fileStatus, null, link);
}

Expand All @@ -199,7 +242,7 @@ public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileSt
* @param reference The reference instance
*/
public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileStatus fileStatus,
final Reference reference) {
final Reference reference) throws IOException {
this(conf, fs, fileStatus, reference, null);
}

Expand All @@ -212,14 +255,15 @@ public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileSt
* @param link The link instance
*/
public StoreFileInfo(final Configuration conf, final FileSystem fs, final FileStatus fileStatus,
final Reference reference, final HFileLink link) {
final Reference reference, final HFileLink link) throws IOException {
this.fs = fs;
this.conf = conf;
this.primaryReplica = false;
this.initialPath = (fileStatus == null) ? null : fileStatus.getPath();
this.createdTimestamp = (fileStatus == null) ? 0 : fileStatus.getModificationTime();
this.reference = reference;
this.link = link;
this.initialPath = (fileStatus == null) ? null : fileStatus.getPath();
this.createdTimestamp = (fileStatus == null) ? 0 : fileStatus.getModificationTime();
this.localStatus = (fileStatus == null) ? loadAndCacheFileStatus(fs) : fileStatus;
this.noReadahead = this.conf.getBoolean(STORE_FILE_READER_NO_READAHEAD,
DEFAULT_STORE_FILE_READER_NO_READAHEAD);
}
Expand Down Expand Up @@ -286,7 +330,6 @@ ReaderContext createReaderContext(boolean doDropBehind, long readahead, ReaderTy
if (this.link != null) {
// HFileLink
in = new FSDataInputStreamWrapper(fs, this.link, doDropBehind, readahead);
status = this.link.getFileStatus(fs);
} else if (this.reference != null) {
// HFile Reference
Path referencePath = getReferredToFile(this.getPath());
Expand All @@ -300,11 +343,10 @@ 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);
}
status = getFileStatus();
long length = status.getLen();
ReaderContextBuilder contextBuilder =
new ReaderContextBuilder().withInputStreamWrapper(in).withFileSize(length)
Expand Down Expand Up @@ -356,43 +398,10 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS
* @return The {@link FileStatus} of the file referenced by this StoreFileInfo
*/
public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOException {
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;
} else {
// HFile Reference
Path referencePath = getReferredToFile(this.getPath());
status = fs.getFileStatus(referencePath);
}
} 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;
}
}
throw exToThrow;
} else {
status = fs.getFileStatus(initialPath);
}
if(null == this.localStatus) {
return loadAndCacheFileStatus(fs);
}
return status;
return this.localStatus;
}

/** @return The {@link Path} of the file */
Expand Down
9 changes: 5 additions & 4 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.io.IOException;

import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HBaseTestingUtility;
Expand Down Expand Up @@ -90,9 +91,9 @@ public void testEqualsWithLink() throws IOException {
new Path(mob, "f1"), new Path(archive, "f1"));

StoreFileInfo info1 = new StoreFileInfo(TEST_UTIL.getConfiguration(),
TEST_UTIL.getTestFileSystem(), null, link1);
TEST_UTIL.getTestFileSystem(), new FileStatus(), link1);
StoreFileInfo info2 = new StoreFileInfo(TEST_UTIL.getConfiguration(),
TEST_UTIL.getTestFileSystem(), null, link2);
TEST_UTIL.getTestFileSystem(), new FileStatus(), link2);

assertEquals(info1, info2);
assertEquals(info1.hashCode(), info2.hashCode());
Expand All @@ -104,8 +105,8 @@ public void testOpenErrorMessageHFileLink() throws IOException, IllegalStateExce
// Try to open nonsense hfilelink. Make sure exception is from HFileLink.
Path p = new Path("/hbase/test/0123/cf/testtb=4567-abcd");
try (FileSystem fs = FileSystem.get(TEST_UTIL.getConfiguration())) {
StoreFileInfo sfi = new StoreFileInfo(TEST_UTIL.getConfiguration(), fs, p, true);
try {
StoreFileInfo sfi = new StoreFileInfo(TEST_UTIL.getConfiguration(), fs, p, true);
ReaderContext context = sfi.createReaderContext(false, 1000, ReaderType.PREAD);
sfi.createReader(context, null);
throw new IllegalStateException();
Expand All @@ -124,8 +125,8 @@ public void testOpenErrorMessageReference() throws IOException {
fs.mkdirs(p.getParent());
Reference r = Reference.createBottomReference(HConstants.EMPTY_START_ROW);
r.write(fs, p);
StoreFileInfo sfi = new StoreFileInfo(TEST_UTIL.getConfiguration(), fs, p, true);
try {
StoreFileInfo sfi = new StoreFileInfo(TEST_UTIL.getConfiguration(), fs, p, true);
ReaderContext context = sfi.createReaderContext(false, 1000, ReaderType.PREAD);
sfi.createReader(context, null);
throw new IllegalStateException();
Expand Down