Skip to content
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

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

Closed
wants to merge 8 commits into from
Closed

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

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a41081d
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Dec 23, 2019
6800ae1
Update StoreFileInfo.java
HuiHang-Yu Dec 24, 2019
8cbca97
Update StoreFileInfo.java
HuiHang-Yu Dec 24, 2019
b122afd
Update StoreFileInfo.java
HuiHang-Yu Jan 2, 2020
cfa10c5
Update StoreFileInfo.java
HuiHang-Yu Jan 3, 2020
0948726
initialize localStatus on construction
HuiHang-Yu Mar 23, 2020
10cd97a
Merge pull request #1 from apache/master
HuiHang-Yu Mar 30, 2020
f1dac8f
Update StoreFileInfo.java
HuiHang-Yu Mar 30, 2020
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
46 changes: 26 additions & 20 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,7 @@
public class StoreFileInfo {
private static final Logger LOG = LoggerFactory.getLogger(StoreFileInfo.class);

private final 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 @@ -97,7 +98,7 @@ public class StoreFileInfo {

private RegionCoprocessorHost coprocessorHost;

// timestamp on when the file was created, is 0 and ignored for reference or link files
// change to use the createdTimestamp of reference or link files , will it create some problem later ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment correct? The createTimeStamp is gotten once only on construction. YOu are not changing the behavior that I can see. Should createTimestamp be made final as in private final long createdTimestamp?

Copy link
Author

@HuiHang-Yu HuiHang-Yu Jan 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It is not necessary to make createdTimestamp to be final i think .

Copy link
Contributor

Choose a reason for hiding this comment

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

Its good to mark data members final if your intent is that they are not ever to change. It helps the compiler and it helps the dev reading the code later.

private long createdTimestamp;

private long size;
Expand Down Expand Up @@ -155,18 +156,20 @@ 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();
} else {
FileStatus fStatus = fs.getFileStatus(initialPath);
this.createdTimestamp = fStatus.getModificationTime();
this.size = fStatus.getLen();
this.localStatus = fileStatus;
}else{
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");
}
if(this.localStatus == null ){
this.localStatus = loadAndCacheFileStatus(fs);
}
}

/**
Expand Down Expand Up @@ -286,7 +289,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 +302,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 @@ -349,21 +350,15 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS
return FSUtils.computeHDFSBlocksDistribution(fs, status, 0, status.getLen());
}
}

/**
* Get the {@link FileStatus} of the file referenced by this StoreFileInfo
* @param fs The current file system to use.
* @return The {@link FileStatus} of the file referenced by this StoreFileInfo
*/
public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOException {
FileStatus status;
private FileStatus loadAndCacheFileStatus(final FileSystem fs) throws IOException {
FileStatus status = null ;
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);
status = link.getFileStatus(fs);
} catch (FileNotFoundException ex) {
// try the other location
exToThrow = ex;
Expand All @@ -381,7 +376,7 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio
for (int i = 0; i < this.link.getLocations().length; i++) {
// HFileLink
try {
return link.getFileStatus(fs);
status = link.getFileStatus(fs);
} catch (FileNotFoundException ex) {
// try the other location
exToThrow = ex;
Expand All @@ -394,6 +389,17 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio
}
return status;
}
/**
* Get the {@link FileStatus} of the file referenced by this StoreFileInfo
* @param fs The current file system to use.
* @return The {@link FileStatus} of the file referenced by this StoreFileInfo
*/
public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOException {
if(null == this.localStatus){
throw new IOException("localStatus is not initialize on construct");
}
return this.localStatus;
}

/** @return The {@link Path} of the file */
public Path getPath() {
Expand Down