From a41081d854bbe5a623de5d2272373b2d3032aa27 Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Mon, 23 Dec 2019 11:42:53 +0800 Subject: [PATCH 1/7] HBASE-23584 : Descrease rpc getFileStatus count when open a storefile --- .../hbase/regionserver/StoreFileInfo.java | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) 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 15ed359a8822..4d963ef54450 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 @@ -51,6 +51,7 @@ public class StoreFileInfo { private static final Logger LOG = LoggerFactory.getLogger(StoreFileInfo.class); + private FileStatus localStatus; /** * 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. @@ -108,6 +109,7 @@ public class StoreFileInfo { private RegionCoprocessorHost coprocessorHost; // timestamp on when the file was created, is 0 and ignored for reference or link files + // the before timestamp is shown as above ! Now i change to use the createdTimestamp of reference or link files , will it create some problem later ? private long createdTimestamp; private long size; @@ -168,9 +170,8 @@ private StoreFileInfo(final Configuration conf, final FileSystem fs, final FileS this.createdTimestamp = fileStatus.getModificationTime(); this.size = fileStatus.getLen(); } else { - FileStatus fStatus = fs.getFileStatus(initialPath); - this.createdTimestamp = fStatus.getModificationTime(); - this.size = fStatus.getLen(); + this.createdTimestamp = this.getFileStatus().getModificationTime(); + this.size = this.getFileStatus().getLen(); } this.reference = null; this.link = null; @@ -296,7 +297,7 @@ 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); + // status = this.link.getFileStatus(fs); } else if (this.reference != null) { // HFile Reference Path referencePath = getReferredToFile(this.getPath()); @@ -310,11 +311,12 @@ ReaderContext createReaderContext(boolean doDropBehind, long readahead, ReaderTy newFnfe.initCause(fnfe); throw newFnfe; } - status = fs.getFileStatus(referencePath); + // status = fs.getFileStatus(referencePath); } else { in = new FSDataInputStreamWrapper(fs, this.getPath(), doDropBehind, readahead); - status = fs.getFileStatus(initialPath); + // status = fs.getFileStatus(initialPath); } + status = this.getFileStatus(); long length = status.getLen(); ReaderContextBuilder contextBuilder = new ReaderContextBuilder() .withInputStreamWrapper(in) @@ -370,13 +372,16 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS */ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOException { FileStatus status; + if(this.localStatus !=null) return this.localStatus; 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); + this.localStatus = link.getFileStatus(fs); + status = this.localStatus; + return status; } catch (FileNotFoundException ex) { // try the other location exToThrow = ex; @@ -386,7 +391,8 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio } else { // HFile Reference Path referencePath = getReferredToFile(this.getPath()); - status = fs.getFileStatus(referencePath); + this.localStatus = fs.getFileStatus(referencePath); + status = this.localStatus; } } else { if (this.link != null) { @@ -394,7 +400,9 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio for (int i = 0; i < this.link.getLocations().length; i++) { // HFileLink try { - return link.getFileStatus(fs); + this.localStatus = link.getFileStatus(fs); + status = this.localStatus; + return status; } catch (FileNotFoundException ex) { // try the other location exToThrow = ex; @@ -402,7 +410,8 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio } throw exToThrow; } else { - status = fs.getFileStatus(initialPath); + this.localStatus = fs.getFileStatus(initialPath); + status = this.localStatus; } } return status; From 6800ae14b292845321106ef8d151e6d33b172ee9 Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Tue, 24 Dec 2019 16:04:32 +0800 Subject: [PATCH 2/7] Update StoreFileInfo.java --- .../hadoop/hbase/regionserver/StoreFileInfo.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) 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 4d963ef54450..b411e10c17e1 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 @@ -108,8 +108,7 @@ public class StoreFileInfo { private RegionCoprocessorHost coprocessorHost; - // timestamp on when the file was created, is 0 and ignored for reference or link files - // the before timestamp is shown as above ! Now i change to use the createdTimestamp of reference or link files , will it create some problem later ? + // change to use the createdTimestamp of reference or link files , will it create some problem later ? private long createdTimestamp; private long size; @@ -297,7 +296,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()); @@ -311,10 +309,8 @@ 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 = this.getFileStatus(); long length = status.getLen(); @@ -364,7 +360,7 @@ 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. @@ -372,7 +368,7 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS */ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOException { FileStatus status; - if(this.localStatus !=null) return this.localStatus; + if(this.localStatus != null) {return this.localStatus;} if (this.reference != null) { if (this.link != null) { FileNotFoundException exToThrow = null; From 8cbca976713edaf934553e426b66768660131605 Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Tue, 24 Dec 2019 16:10:23 +0800 Subject: [PATCH 3/7] Update StoreFileInfo.java --- .../apache/hadoop/hbase/regionserver/StoreFileInfo.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 b411e10c17e1..5c41923437f1 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 @@ -360,7 +360,7 @@ 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. @@ -376,8 +376,7 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio // HFileLink Reference try { this.localStatus = link.getFileStatus(fs); - status = this.localStatus; - return status; + return this.localStatus; } catch (FileNotFoundException ex) { // try the other location exToThrow = ex; @@ -397,8 +396,7 @@ public FileStatus getReferencedFileStatus(final FileSystem fs) throws IOExceptio // HFileLink try { this.localStatus = link.getFileStatus(fs); - status = this.localStatus; - return status; + return this.localStatus; } catch (FileNotFoundException ex) { // try the other location exToThrow = ex; From b122afd2a6fc28bfef59c33917f319486c4111fd Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Thu, 2 Jan 2020 16:52:48 +0800 Subject: [PATCH 4/7] Update StoreFileInfo.java --- .../org/apache/hadoop/hbase/regionserver/StoreFileInfo.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 5c41923437f1..50a544a3177e 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 @@ -169,8 +169,8 @@ private StoreFileInfo(final Configuration conf, final FileSystem fs, final FileS this.createdTimestamp = fileStatus.getModificationTime(); this.size = fileStatus.getLen(); } else { - this.createdTimestamp = this.getFileStatus().getModificationTime(); - this.size = this.getFileStatus().getLen(); + this.createdTimestamp = getFileStatus().getModificationTime(); + this.size = getFileStatus().getLen(); } this.reference = null; this.link = null; @@ -360,7 +360,7 @@ 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. From cfa10c56ea063350327932e2184ffa09b7278221 Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Fri, 3 Jan 2020 14:55:03 +0800 Subject: [PATCH 5/7] Update StoreFileInfo.java --- .../org/apache/hadoop/hbase/regionserver/StoreFileInfo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 50a544a3177e..a725d15ee10f 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 @@ -51,7 +51,7 @@ public class StoreFileInfo { private static final Logger LOG = LoggerFactory.getLogger(StoreFileInfo.class); - private FileStatus localStatus; + 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. From 0948726dad79ee41fb777acaede430a835e81ce1 Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Mon, 23 Mar 2020 11:40:23 +0800 Subject: [PATCH 6/7] initialize localStatus on construction initialize localStatus on construction --- .../hbase/regionserver/StoreFileInfo.java | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) 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 a725d15ee10f..d8bc73e6dd6c 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 @@ -166,17 +166,20 @@ private StoreFileInfo(final Configuration conf, final FileSystem fs, final FileS } else if (isHFile(p)) { // HFile if (fileStatus != null) { - this.createdTimestamp = fileStatus.getModificationTime(); - this.size = fileStatus.getLen(); - } else { - this.createdTimestamp = getFileStatus().getModificationTime(); - this.size = getFileStatus().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); + } } /** @@ -312,7 +315,7 @@ ReaderContext createReaderContext(boolean doDropBehind, long readahead, ReaderTy } else { in = new FSDataInputStreamWrapper(fs, this.getPath(), doDropBehind, readahead); } - status = this.getFileStatus(); + status = getFileStatus(); long length = status.getLen(); ReaderContextBuilder contextBuilder = new ReaderContextBuilder() .withInputStreamWrapper(in) @@ -360,15 +363,7 @@ 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; - if(this.localStatus != null) {return this.localStatus;} + private FileStatus loadAndCacheFileStatus(final FileSystem fs) throws IOException { if (this.reference != null) { if (this.link != null) { FileNotFoundException exToThrow = null; @@ -410,6 +405,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() { From f1dac8f236be45af7fb71ff5ef5f81371170a12a Mon Sep 17 00:00:00 2001 From: yuhuiyang Date: Mon, 30 Mar 2020 15:20:27 +0800 Subject: [PATCH 7/7] Update StoreFileInfo.java --- .../hadoop/hbase/regionserver/StoreFileInfo.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) 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 58176b08cd6a..dc8f83384e23 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 @@ -351,14 +351,14 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS } } 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 { - this.localStatus = link.getFileStatus(fs); - return this.localStatus; + status = link.getFileStatus(fs); } catch (FileNotFoundException ex) { // try the other location exToThrow = ex; @@ -368,8 +368,7 @@ private FileStatus loadAndCacheFileStatus(final FileSystem fs) throws IOExceptio } else { // HFile Reference Path referencePath = getReferredToFile(this.getPath()); - this.localStatus = fs.getFileStatus(referencePath); - status = this.localStatus; + status = fs.getFileStatus(referencePath); } } else { if (this.link != null) { @@ -377,8 +376,7 @@ private FileStatus loadAndCacheFileStatus(final FileSystem fs) throws IOExceptio for (int i = 0; i < this.link.getLocations().length; i++) { // HFileLink try { - this.localStatus = link.getFileStatus(fs); - return this.localStatus; + status = link.getFileStatus(fs); } catch (FileNotFoundException ex) { // try the other location exToThrow = ex; @@ -386,8 +384,7 @@ private FileStatus loadAndCacheFileStatus(final FileSystem fs) throws IOExceptio } throw exToThrow; } else { - this.localStatus = fs.getFileStatus(initialPath); - status = this.localStatus; + status = fs.getFileStatus(initialPath); } } return status;