You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2019/12/23 03:47:28 UTC
[GitHub] [hbase] HuiHang-Yu opened a new pull request #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
HuiHang-Yu opened a new pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958
https://issues.apache.org/jira/browse/HBASE-23584
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361073108
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -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);
Review comment:
Remove rather than comment-out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu closed pull request #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu closed pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-568785624
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 31s | master passed |
| +1 :green_heart: | compile | 0m 55s | master passed |
| +1 :green_heart: | checkstyle | 1m 20s | master passed |
| +1 :green_heart: | shadedjars | 4m 36s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
| +0 :ok: | spotbugs | 4m 27s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 59s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -1 :x: | checkstyle | 1m 18s | hbase-server: The patch generated 2 new + 15 unchanged - 0 fixed = 17 total (was 15) |
| -1 :x: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | shadedjars | 4m 40s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 15m 43s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
| +1 :green_heart: | findbugs | 4m 29s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 283m 45s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 30s | The patch does not generate ASF License warnings. |
| | | 342m 0s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.client.TestSnapshotTemporaryDirectoryWithRegionReplicas |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 486d9d1e4400 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-958/out/precommit/personality/provided.sh |
| git revision | master / ee19008b12 |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/3/artifact/out/diff-checkstyle-hbase-server.txt |
| whitespace | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/3/artifact/out/whitespace-eol.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/3/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/3/testReport/ |
| Max. process+thread count | 4900 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/3/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361737467
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -168,9 +169,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();
Review comment:
Generally, we do not put the 'this.' in front of local method invocation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-568390071
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 48s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 57s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | checkstyle | 1m 30s | master passed |
| +1 :green_heart: | shadedjars | 5m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
| +0 :ok: | spotbugs | 4m 47s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 44s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| -1 :x: | checkstyle | 1m 59s | hbase-server: The patch generated 2 new + 15 unchanged - 0 fixed = 17 total (was 15) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 46s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 20s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
| +1 :green_heart: | findbugs | 4m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 163m 57s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 230m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 6a8953508ce1 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-958/out/precommit/personality/provided.sh |
| git revision | master / 4b6ce0fcbf |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/1/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/1/testReport/ |
| Max. process+thread count | 4524 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/1/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-602369657
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 0s | Docker mode activated. |
| -1 :x: | patch | 0m 3s | https://github.com/apache/hbase/pull/958 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help. |
| Subsystem | Report/Notes |
|----------:|:-------------|
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/6/console |
| versions | git=2.17.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361088206
##########
File path: 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;
Review comment:
Now it will be updated when reopened only because hfile will not be modified after writen i think . Is there some scenarios fileStatus will be modified?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361737911
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -362,21 +360,23 @@ 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;}
Review comment:
Do you think multiple threads will come in h ere at same time? Maybe it would be better to make localStatus be final and assign it on construction of this StoreFileInfo? If so, change name localStatus to status?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362647777
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -168,9 +169,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 = getFileStatus().getModificationTime();
Review comment:
Seems odd that we set modification time to created but that is not of your making; it was there before you came along (perhaps only modification 'works' or this is more trustworthy than create time...)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361091633
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -386,23 +391,27 @@ 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) {
FileNotFoundException exToThrow = null;
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;
}
}
throw exToThrow;
} else {
- status = fs.getFileStatus(initialPath);
+ this.localStatus = fs.getFileStatus(initialPath);
+ status = this.localStatus;
Review comment:
That's a good idea !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362397347
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -107,7 +108,7 @@
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 ?
Review comment:
It is not necessary to make createdTimestamp to be final i think .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361073438
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -386,23 +391,27 @@ 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) {
FileNotFoundException exToThrow = null;
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;
Review comment:
This code is repeated. Put it into a method?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HorizonNet commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HorizonNet commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361978568
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -362,21 +360,23 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS
return FSUtils.computeHDFSBlocksDistribution(fs, status, 0, status.getLen());
}
}
-
+
Review comment:
Seems like there is a little space added. Could you please remove it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361737332
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -107,7 +108,7 @@
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 ?
Review comment:
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?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-602369656
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 0s | Docker mode activated. |
| -1 :x: | patch | 0m 3s | https://github.com/apache/hbase/pull/958 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help. |
| Subsystem | Report/Notes |
|----------:|:-------------|
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/6/console |
| versions | git=2.17.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361519440
##########
File path: 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;
Review comment:
Not that I can think of.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-570194268
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 27s | master passed |
| +1 :green_heart: | compile | 0m 56s | master passed |
| +1 :green_heart: | checkstyle | 1m 21s | master passed |
| +1 :green_heart: | shadedjars | 4m 36s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
| +0 :ok: | spotbugs | 4m 19s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 17s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 2s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| -1 :x: | checkstyle | 1m 18s | hbase-server: The patch generated 2 new + 15 unchanged - 0 fixed = 17 total (was 15) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 41s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 15m 46s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
| +1 :green_heart: | findbugs | 4m 32s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 158m 2s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 35s | The patch does not generate ASF License warnings. |
| | | 216m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux e2b44a1a7a1b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-958/out/precommit/personality/provided.sh |
| git revision | master / 2a4bd0574b |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/4/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/4/testReport/ |
| Max. process+thread count | 4526 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/4/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361089869
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -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);
Review comment:
Oh my fault ! I will remove it later !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361073454
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -386,23 +391,27 @@ 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) {
FileNotFoundException exToThrow = null;
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;
}
}
throw exToThrow;
} else {
- status = fs.getFileStatus(initialPath);
+ this.localStatus = fs.getFileStatus(initialPath);
+ status = this.localStatus;
Review comment:
Three times.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362646657
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -362,21 +360,23 @@ 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;}
Review comment:
If so, make it final then we are explicit that it does not change during life time of a StoreFileInfo instance and we are clear about its thread safety. If done in constructor, you don't need this null check then either?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361073332
##########
File path: 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;
Review comment:
Does it need to be volatile? Will StoreFileInfo be accessed by many threads? Can it be final/created on construction?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361073208
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -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;
Review comment:
Need parens as per convention in the code that surrounds this. Needs space after the '='.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361088206
##########
File path: 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;
Review comment:
Now it will be updated when reopened only because hfile will not be modified after write i think . Is there some scenarios fileStatus will be modified?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361072493
##########
File path: 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;
Review comment:
I like idea of caching status. When does it get invalidated?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361088206
##########
File path: 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;
Review comment:
Now it will be updated when reopened only because hfile will not be modified after write i think .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-568734758
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 22s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 4s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | checkstyle | 1m 31s | master passed |
| +1 :green_heart: | shadedjars | 5m 1s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
| +0 :ok: | spotbugs | 5m 39s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 5m 32s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 13s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| -1 :x: | checkstyle | 1m 28s | hbase-server: The patch generated 2 new + 15 unchanged - 0 fixed = 17 total (was 15) |
| -1 :x: | whitespace | 0m 0s | The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | shadedjars | 5m 27s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 21m 48s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
| +1 :green_heart: | findbugs | 4m 56s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 166m 10s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 29s | The patch does not generate ASF License warnings. |
| | | 236m 21s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hbase.quotas.TestClusterScopeQuotaThrottle |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 4e40f8e19f73 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-958/out/precommit/personality/provided.sh |
| git revision | master / fc15ea7546 |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/2/artifact/out/diff-checkstyle-hbase-server.txt |
| whitespace | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/2/artifact/out/whitespace-eol.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/2/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/2/testReport/ |
| Max. process+thread count | 4483 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/2/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361072664
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -108,6 +109,7 @@
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 ?
Review comment:
Where is 'above'. Comments don't usually refer to 'i' since code is product of many. What is the problem this will create and if you don't know what it might be, why change i?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362397347
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -107,7 +108,7 @@
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 ?
Review comment:
It is ok to make createdTimestamp to be final i think.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362648386
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -362,21 +360,23 @@ 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;}
Review comment:
Couldn't you simplify this method if you fetched status on construction (using the bulk of this method but taking it private/internal?)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362397501
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -362,21 +360,23 @@ private HDFSBlocksDistribution computeHDFSBlocksDistributionInternal(final FileS
return FSUtils.computeHDFSBlocksDistribution(fs, status, 0, status.getLen());
}
}
-
+
Review comment:
Sorry for my fault !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362397457
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -168,9 +169,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();
Review comment:
I will remove it .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r361091318
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -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;
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362647394
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -107,7 +108,7 @@
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 ?
Review comment:
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.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-570490860
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 3s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 32s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 19s | master passed |
| +1 :green_heart: | shadedjars | 4m 35s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
| +0 :ok: | spotbugs | 4m 27s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 24s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 28s | root in the patch failed. |
| -1 :x: | compile | 0m 38s | hbase-server in the patch failed. |
| -1 :x: | javac | 0m 38s | hbase-server in the patch failed. |
| -1 :x: | checkstyle | 1m 15s | hbase-server: The patch generated 2 new + 15 unchanged - 0 fixed = 17 total (was 15) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | shadedjars | 3m 27s | patch has 20 errors when building our shaded downstream artifacts. |
| -1 :x: | hadoopcheck | 1m 48s | The patch causes 20 errors with Hadoop v2.8.5. |
| -1 :x: | hadoopcheck | 3m 41s | The patch causes 20 errors with Hadoop v2.9.2. |
| -1 :x: | hadoopcheck | 5m 37s | The patch causes 20 errors with Hadoop v3.1.2. |
| +1 :green_heart: | javadoc | 0m 33s | the patch passed |
| -1 :x: | findbugs | 0m 37s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 39s | hbase-server in the patch failed. |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 35m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 17267e7d85f3 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-958/out/precommit/personality/provided.sh |
| git revision | master / ccfbdadb0f |
| Default Java | 1.8.0_181 |
| mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-mvninstall-root.txt |
| compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-compile-hbase-server.txt |
| javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-compile-hbase-server.txt |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/diff-checkstyle-hbase-server.txt |
| shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-shadedjars.txt |
| hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-javac-2.8.5.txt |
| hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-javac-2.9.2.txt |
| hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-javac-3.1.2.txt |
| findbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-findbugs-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/artifact/out/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/testReport/ |
| Max. process+thread count | 93 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/5/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] HuiHang-Yu commented on a change in pull request #958:
HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
HuiHang-Yu commented on a change in pull request #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#discussion_r362402047
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
##########
@@ -362,21 +360,23 @@ 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;}
Review comment:
LocalStatus will be assigned when storeFileInfo construction is invoked . Will it be some problem happend when multiple threads will come in here at the same time ? So it is not necessary to make localstatus to be final and volatile i think .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #958: HBASE-23584 :
Descrease rpc getFileStatus count when open a storefile
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #958: HBASE-23584 : Descrease rpc getFileStatus count when open a storefile
URL: https://github.com/apache/hbase/pull/958#issuecomment-602369643
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 0s | Docker mode activated. |
| -1 :x: | patch | 0m 3s | https://github.com/apache/hbase/pull/958 does not apply to master. Rebase required? Wrong Branch? See https://yetus.apache.org/documentation/in-progress/precommit-patchnames for help. |
| Subsystem | Report/Notes |
|----------:|:-------------|
| GITHUB PR | https://github.com/apache/hbase/pull/958 |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-958/6/console |
| versions | git=2.17.1 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services