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 2020/01/11 07:14:58 UTC
[GitHub] [hbase] Apache9 opened a new pull request #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Apache9 opened a new pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022
----------------------------------------------------------------
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 issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
saintstack commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574890916
Filed HBASE-23697 for doc on RegionProcedureStore.
----------------------------------------------------------------
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 #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-575142947
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 7s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 7 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 44s | master passed |
| +1 :green_heart: | compile | 1m 39s | master passed |
| +1 :green_heart: | checkstyle | 2m 9s | master passed |
| +0 :ok: | refguide | 6m 26s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | shadedjars | 5m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 11s | master passed |
| +0 :ok: | spotbugs | 5m 2s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 6m 22s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 28s | the patch passed |
| +1 :green_heart: | compile | 1m 38s | the patch passed |
| +1 :green_heart: | javac | 1m 38s | the patch passed |
| -1 :x: | checkstyle | 1m 30s | hbase-server: The patch generated 3 new + 103 unchanged - 0 fixed = 106 total (was 103) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 6m 17s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | shadedjars | 5m 4s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 14s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 1m 10s | the patch passed |
| +1 :green_heart: | findbugs | 6m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 3m 6s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 3m 36s | hbase-procedure in the patch passed. |
| +1 :green_heart: | unit | 155m 54s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 1m 16s | The patch does not generate ASF License warnings. |
| | | 247m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1022 |
| Optional Tests | dupname asflicense javac javadoc unit refguide xml spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux b929b09c26eb 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1022/out/precommit/personality/provided.sh |
| git revision | master / 19d3bed1d4 |
| Default Java | 1.8.0_181 |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/5/artifact/out/branch-site/book.html |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/5/artifact/out/diff-checkstyle-hbase-server.txt |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/5/artifact/out/patch-site/book.html |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/5/testReport/ |
| Max. process+thread count | 5002 (vs. ulimit of 10000) |
| modules | C: hbase-common hbase-procedure hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/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] saintstack commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r368174200
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1520,8 +1518,10 @@ protected void stopServiceThreads() {
private void createProcedureExecutor() throws IOException {
MasterProcedureEnv procEnv = new MasterProcedureEnv(this);
- procedureStore =
- new RegionProcedureStore(this, new MasterProcedureEnv.FsUtilsLeaseRecovery(this));
+ // Create cleaner thread pool
+ cleanerPool = new DirScanPool(conf);
Review comment:
Strange place to start cleanerPool down here in the procedure startup given it is used cleaning WALs and hfile...
----------------------------------------------------------------
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] busbey commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574289502
> On why we put all things on the WAL filesystem, partly because that I want to put all the data under a single directory, which can hide the internal implementation details about procedure store, and easy to replace the old implementation with another implementation.
>
> And another thing is about performance. The WAL filesystem is HDFS, and the HFile storage, if not the same one, is usually S3 or some other object storages, which are very slow, usually. And usually the size of the procedure store will be small, I think the cost is also fine.
I like that this goes on the WAL filesystem and I think the justification is good. I went to check the docs to make sure the reasoning is spelled out as well there. Looks like the ref guide does not have details about the new procedure store and still has a bunch of stuff about the old one. Is there a JIRA I can watch to review when those docs are up?
----------------------------------------------------------------
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] Apache9 commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r368254586
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1520,8 +1518,10 @@ protected void stopServiceThreads() {
private void createProcedureExecutor() throws IOException {
MasterProcedureEnv procEnv = new MasterProcedureEnv(this);
- procedureStore =
- new RegionProcedureStore(this, new MasterProcedureEnv.FsUtilsLeaseRecovery(this));
+ // Create cleaner thread pool
+ cleanerPool = new DirScanPool(conf);
Review comment:
The startServiceThreads method is called after createProcedureExecutor, notice that, it is create, not start...
----------------------------------------------------------------
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 #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-575314621
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 7 new or modified test files. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 41s | master passed |
| +1 :green_heart: | compile | 1m 37s | master passed |
| +1 :green_heart: | checkstyle | 2m 9s | master passed |
| +0 :ok: | refguide | 6m 26s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | shadedjars | 4m 59s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | master passed |
| +0 :ok: | spotbugs | 4m 49s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 6m 9s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 28s | the patch passed |
| +1 :green_heart: | compile | 1m 38s | the patch passed |
| +1 :green_heart: | javac | 1m 38s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 7s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 6m 19s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | shadedjars | 5m 2s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 8s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 1m 11s | the patch passed |
| +1 :green_heart: | findbugs | 6m 34s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 3m 8s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 3m 35s | hbase-procedure in the patch passed. |
| +1 :green_heart: | unit | 155m 19s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 1m 9s | The patch does not generate ASF License warnings. |
| | | 246m 4s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1022 |
| Optional Tests | dupname asflicense javac javadoc unit refguide xml spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 608fe92c9c22 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1022/out/precommit/personality/provided.sh |
| git revision | master / edc53683c4 |
| Default Java | 1.8.0_181 |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/6/artifact/out/branch-site/book.html |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/6/artifact/out/patch-site/book.html |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/6/testReport/ |
| Max. process+thread count | 4602 (vs. ulimit of 10000) |
| modules | C: hbase-common hbase-procedure hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/6/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 #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r368231529
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1520,8 +1518,10 @@ protected void stopServiceThreads() {
private void createProcedureExecutor() throws IOException {
MasterProcedureEnv procEnv = new MasterProcedureEnv(this);
- procedureStore =
- new RegionProcedureStore(this, new MasterProcedureEnv.FsUtilsLeaseRecovery(this));
+ // Create cleaner thread pool
+ cleanerPool = new DirScanPool(conf);
Review comment:
I was thinking you'd call it up in the caller's method, in startServiceThreads, rather than down hidden in here in startProcedureExecutor... i.e. move the line up thee lines from where it was.
----------------------------------------------------------------
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] busbey commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574926547
Correct; the docs are a mess.
----------------------------------------------------------------
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] Apache9 commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574195642
Ping @saintstack
----------------------------------------------------------------
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] Apache9 commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r367178351
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +156,43 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private boolean isHFileDeleteable(FileStatus status) {
+ long currentTime = EnvironmentEdgeManager.currentTime();
+ long time = status.getModificationTime();
+ long life = currentTime - time;
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("HFile life: {}ms, ttl: {}ms, current: {}, from: {}", life, compactedHFileTTLMs,
+ currentTime, time);
+ }
+ if (life < 0) {
+ LOG.warn("Found a hfile ({}) newer than current time ({} < {}), probably a clock skew",
+ status.getPath(), currentTime, time);
+ return false;
+ }
+ return life > compactedHFileTTLMs;
+ }
+
+ void cleanupCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ // our HFiles are on the WAL file system, but the global HFile archive directory is on the
+ // root(HFile) file system, we can not move between two different file systems. So here we have
+ // to implement our own TTL cleaner.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+
+ for (FileStatus status : fs.listStatus(archiveDir)) {
+ if (isHFileDeleteable(status)) {
Review comment:
Maybe a bit overkill? AFAIK, the only suitable HFileCleaner for this region is the TTL one, others like Links or Back Ref are not needed. Just saying. If you think this is the correct way then I could pass the ChoreService in and create a cleaner chain for cleaning the hfiles.
----------------------------------------------------------------
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 #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-573433139
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 4s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 56s | master passed |
| +1 :green_heart: | compile | 0m 59s | 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 | 4m 49s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 47s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 37s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 28s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 59s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 6s | 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 | 5m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 154m 48s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 26s | The patch does not generate ASF License warnings. |
| | | 217m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1022 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux d13a4b4c0e5b 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/Base-PreCommit-GitHub-PR_PR-1022/out/precommit/personality/provided.sh |
| git revision | master / 4ad12e03b8 |
| Default Java | 1.8.0_181 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/4/testReport/ |
| Max. process+thread count | 4915 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/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] Apache9 commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-575049206
Added a hbase.procedure.store.region.hfilecleaner.plugins, and add the default config to hbase-default.xml. The default value is org.apache.hadoop.hbase.master.cleaner.TimeToLiveHFileCleaner.
----------------------------------------------------------------
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] busbey commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r367181321
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +156,43 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private boolean isHFileDeleteable(FileStatus status) {
+ long currentTime = EnvironmentEdgeManager.currentTime();
+ long time = status.getModificationTime();
+ long life = currentTime - time;
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("HFile life: {}ms, ttl: {}ms, current: {}, from: {}", life, compactedHFileTTLMs,
+ currentTime, time);
+ }
+ if (life < 0) {
+ LOG.warn("Found a hfile ({}) newer than current time ({} < {}), probably a clock skew",
+ status.getPath(), currentTime, time);
+ return false;
+ }
+ return life > compactedHFileTTLMs;
+ }
+
+ void cleanupCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ // our HFiles are on the WAL file system, but the global HFile archive directory is on the
+ // root(HFile) file system, we can not move between two different file systems. So here we have
+ // to implement our own TTL cleaner.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+
+ for (FileStatus status : fs.listStatus(archiveDir)) {
+ if (isHFileDeleteable(status)) {
Review comment:
No not overkill. This is the specific mechanism I mentioned needing to feel comfortable with this landing in branches where I will probably have to support deployments.
I agree that the TTL cleaner is the only one needed by default.
At some point something will go wrong where I need to avoid things getting cleaned out of archive. Being able to reuse a cleaner delegate that normally is used with HFiles will make that easier. It's exactly the kind of operational reuse that makes this implementation shift attractive.
----------------------------------------------------------------
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] Apache9 merged pull request #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 merged pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022
----------------------------------------------------------------
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] busbey commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574718064
> I checked the ref guide but did not seen anything about the procedure store? It is just about the procedure v2 framework, and these thing are not changed...
http://hbase.apache.org/book.html#master.wal
----------------------------------------------------------------
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 #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r365531590
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +146,30 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private void deleteCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ store.closeAndArchiveCompactedFiles();
+ // for now we just deleted these HFiles, without moving them to the global archive directory.
+ // This is because that, our HFiles are on the WAL file system, but the global HFile archive
+ // directory is on the root(HFile) file system, we can not move between two different file
+ // systems.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+ if (LOG.isDebugEnabled()) {
+ FileStatus[] toDelete = fs.listStatus(archiveDir);
+ if (toDelete != null && toDelete.length > 0) {
+ LOG.debug("Delete all archived HFiles under {}: {}", archiveDir, Stream.of(toDelete)
+ .map(s -> s.getPath().getName()).collect(Collectors.joining(", ", "[", "]")));
+ }
+ }
+ fs.delete(archiveDir, true);
+ }
+
private void compact() {
try {
region.compact(true);
+ deleteCompactedHFiles();
Review comment:
We just delete archived files? Why even archive them then? Can we delay the delete at least?
----------------------------------------------------------------
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 #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r365531691
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +146,30 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private void deleteCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ store.closeAndArchiveCompactedFiles();
+ // for now we just deleted these HFiles, without moving them to the global archive directory.
+ // This is because that, our HFiles are on the WAL file system, but the global HFile archive
+ // directory is on the root(HFile) file system, we can not move between two different file
+ // systems.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+ if (LOG.isDebugEnabled()) {
+ FileStatus[] toDelete = fs.listStatus(archiveDir);
+ if (toDelete != null && toDelete.length > 0) {
+ LOG.debug("Delete all archived HFiles under {}: {}", archiveDir, Stream.of(toDelete)
+ .map(s -> s.getPath().getName()).collect(Collectors.joining(", ", "[", "]")));
+ }
+ }
+ fs.delete(archiveDir, true);
+ }
+
private void compact() {
try {
region.compact(true);
+ deleteCompactedHFiles();
Review comment:
Why we writing hfiles into WAL fs out of interest?
----------------------------------------------------------------
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 #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-573307463
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 8s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 6m 2s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | checkstyle | 1m 30s | master passed |
| +1 :green_heart: | shadedjars | 5m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
| +0 :ok: | spotbugs | 4m 56s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 54s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 31s | 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 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 4s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 21m 15s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 44s | the patch passed |
| +1 :green_heart: | findbugs | 5m 19s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 162m 28s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 232m 6s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1022 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 5005aba1ecec 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/Base-PreCommit-GitHub-PR_PR-1022/out/precommit/personality/provided.sh |
| git revision | master / 2e0edacf72 |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/1/artifact/out/diff-checkstyle-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/1/testReport/ |
| Max. process+thread count | 5234 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/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] Apache9 commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574923653
> > I checked the ref guide but did not seen anything about the procedure store? It is just about the procedure v2 framework, and these thing are not changed...
>
> http://hbase.apache.org/book.html#master.wal
Oh, it is not under the section of proc-v2? ...
----------------------------------------------------------------
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] Apache9 commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-575662440
Ping @busbey @saintstack
----------------------------------------------------------------
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] Apache9 commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r368224482
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1520,8 +1518,10 @@ protected void stopServiceThreads() {
private void createProcedureExecutor() throws IOException {
MasterProcedureEnv procEnv = new MasterProcedureEnv(this);
- procedureStore =
- new RegionProcedureStore(this, new MasterProcedureEnv.FsUtilsLeaseRecovery(this));
+ // Create cleaner thread pool
+ cleanerPool = new DirScanPool(conf);
Review comment:
But we have no choice as we need to use it in the next line...
----------------------------------------------------------------
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 #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-573398278
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 10s | 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. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 55s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | checkstyle | 1m 32s | master passed |
| +1 :green_heart: | shadedjars | 5m 2s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
| +0 :ok: | spotbugs | 4m 59s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 58s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 31s | 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 31s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| -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 1s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 17m 41s | 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 | 5m 5s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 166m 32s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 230m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1022 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux baf803cc327b 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/Base-PreCommit-GitHub-PR_PR-1022/out/precommit/personality/provided.sh |
| git revision | master / 4ad12e03b8 |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/3/artifact/out/diff-checkstyle-hbase-server.txt |
| whitespace | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/3/artifact/out/whitespace-eol.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/3/testReport/ |
| Max. process+thread count | 4967 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/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] Apache-HBase commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-573332214
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 1s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 55s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 29s | 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 58s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 4m 57s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 30s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 30s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 5m 47s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 22m 16s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 42s | the patch passed |
| +1 :green_heart: | findbugs | 5m 44s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 160m 45s | hbase-server in the patch passed. |
| +1 :green_heart: | asflicense | 0m 29s | The patch does not generate ASF License warnings. |
| | | 232m 7s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1022 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux eac4126ecc34 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/Base-PreCommit-GitHub-PR_PR-1022/out/precommit/personality/provided.sh |
| git revision | master / 79e799ab7b |
| Default Java | 1.8.0_181 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/2/testReport/ |
| Max. process+thread count | 4961 (vs. ulimit of 10000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1022/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] Apache9 commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-573384618
Add a TTL config for deleting the archived HFiles.
On why we put all things on the WAL filesystem, partly because that I want to put all the data under a single directory, which can hide the internal implementation details about procedure store, and easy to replace the old implementation with another implementation.
And another thing is about performance. The WAL filesystem is HDFS, and the HFile storage, if not the same one, is usually S3 or some other object storages, which are very slow, usually. And usually the size of the procedure store will be small, I think the cost is also fine.
----------------------------------------------------------------
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] Apache9 commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r366889410
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +156,43 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private boolean isHFileDeleteable(FileStatus status) {
+ long currentTime = EnvironmentEdgeManager.currentTime();
+ long time = status.getModificationTime();
+ long life = currentTime - time;
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("HFile life: {}ms, ttl: {}ms, current: {}, from: {}", life, compactedHFileTTLMs,
+ currentTime, time);
+ }
+ if (life < 0) {
+ LOG.warn("Found a hfile ({}) newer than current time ({} < {}), probably a clock skew",
+ status.getPath(), currentTime, time);
+ return false;
+ }
+ return life > compactedHFileTTLMs;
+ }
+
+ void cleanupCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ // our HFiles are on the WAL file system, but the global HFile archive directory is on the
+ // root(HFile) file system, we can not move between two different file systems. So here we have
+ // to implement our own TTL cleaner.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+
+ for (FileStatus status : fs.listStatus(archiveDir)) {
+ if (isHFileDeleteable(status)) {
Review comment:
We are on a different file system comparing to other HFiles so it is not suitable to ask the HFileCleaner, and it seems strange to test a HFile with WALCleaner?
----------------------------------------------------------------
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] busbey commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r366475845
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +156,43 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private boolean isHFileDeleteable(FileStatus status) {
+ long currentTime = EnvironmentEdgeManager.currentTime();
+ long time = status.getModificationTime();
+ long life = currentTime - time;
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("HFile life: {}ms, ttl: {}ms, current: {}, from: {}", life, compactedHFileTTLMs,
+ currentTime, time);
+ }
+ if (life < 0) {
+ LOG.warn("Found a hfile ({}) newer than current time ({} < {}), probably a clock skew",
+ status.getPath(), currentTime, time);
+ return false;
+ }
+ return life > compactedHFileTTLMs;
+ }
+
+ void cleanupCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ // our HFiles are on the WAL file system, but the global HFile archive directory is on the
+ // root(HFile) file system, we can not move between two different file systems. So here we have
+ // to implement our own TTL cleaner.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+
+ for (FileStatus status : fs.listStatus(archiveDir)) {
+ if (isHFileDeleteable(status)) {
Review comment:
We should be asking the other cleaner delegates if it's okay to delete these files. I could see it being either the WAL cleaner delegates or the HFile Cleaner delegates, but it needs to be one of them.
----------------------------------------------------------------
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] Apache9 commented on issue #1022: HBASE-23680
RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
Apache9 commented on issue #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#issuecomment-574673503
> > On why we put all things on the WAL filesystem, partly because that I want to put all the data under a single directory, which can hide the internal implementation details about procedure store, and easy to replace the old implementation with another implementation.
> > And another thing is about performance. The WAL filesystem is HDFS, and the HFile storage, if not the same one, is usually S3 or some other object storages, which are very slow, usually. And usually the size of the procedure store will be small, I think the cost is also fine.
>
> I like that this goes on the WAL filesystem and I think the justification is good. I went to check the docs to make sure the reasoning is spelled out as well there. Looks like the ref guide does not have details about the new procedure store and still has a bunch of stuff about the old one. Is there a JIRA I can watch to review when those docs are up?
I checked the ref guide but did not seen anything about the procedure store? It is just about the procedure v2 framework, and these thing are not changed...
----------------------------------------------------------------
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] busbey commented on a change in pull request #1022:
HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #1022: HBASE-23680 RegionProcedureStore missing cleaning of hfile archive
URL: https://github.com/apache/hbase/pull/1022#discussion_r366946800
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/procedure2/store/region/RegionFlusherAndCompactor.java
##########
@@ -136,9 +156,43 @@ static void setupConf(Configuration conf) {
flushPerChanges, flushIntervalMs);
}
+ private boolean isHFileDeleteable(FileStatus status) {
+ long currentTime = EnvironmentEdgeManager.currentTime();
+ long time = status.getModificationTime();
+ long life = currentTime - time;
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("HFile life: {}ms, ttl: {}ms, current: {}, from: {}", life, compactedHFileTTLMs,
+ currentTime, time);
+ }
+ if (life < 0) {
+ LOG.warn("Found a hfile ({}) newer than current time ({} < {}), probably a clock skew",
+ status.getPath(), currentTime, time);
+ return false;
+ }
+ return life > compactedHFileTTLMs;
+ }
+
+ void cleanupCompactedHFiles() throws IOException {
+ HStore store = Iterables.getOnlyElement(region.getStores());
+ // our HFiles are on the WAL file system, but the global HFile archive directory is on the
+ // root(HFile) file system, we can not move between two different file systems. So here we have
+ // to implement our own TTL cleaner.
+ Path archiveDir = HFileArchiveUtil.getStoreArchivePath(conf, region.getRegionInfo(),
+ store.getColumnFamilyDescriptor().getName());
+ FileSystem fs = archiveDir.getFileSystem(conf);
+
+ for (FileStatus status : fs.listStatus(archiveDir)) {
+ if (isHFileDeleteable(status)) {
Review comment:
Then we should instantiate our own hfile cleaner delegate chain. Defualt it to the TTL delegate and we can skip a custom impl here as well.
----------------------------------------------------------------
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