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 2021/05/12 14:36:42 UTC
[GitHub] [hbase] frostruan opened a new pull request #3261: remove files from filesCompacting when clear compaction queue
frostruan opened a new pull request #3261:
URL: https://github.com/apache/hbase/pull/3261
When clear compaction queues, we just clear the workQueue of ThreadPoolExecutor, but files in compaction request are still in filesCompacting list. maybe we should clear it also.
https://issues.apache.org/jira/browse/HBASE-25880
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-841428077
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 13s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 17s | master passed |
| +1 :green_heart: | compile | 1m 28s | master passed |
| +1 :green_heart: | shadedjars | 10m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 52s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 1s | the patch passed |
| +1 :green_heart: | compile | 1m 37s | the patch passed |
| +1 :green_heart: | javac | 1m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 59s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 220m 53s | hbase-server in the patch passed. |
| | | 259m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1d173ce44e56 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 0955a7a22e |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/testReport/ |
| Max. process+thread count | 3038 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631697628
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1953,22 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ public void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
+ }
+
+ public List<HStoreFile> getFilesCompacting() {
Review comment:
yes. Thanks for your advise. I will fix 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
[GitHub] [hbase] saintstack commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r633285717
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
Review comment:
Was this fixed then?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ // for system compaction, files selection will be delayed until the compaction task
+ // actually runs, so compaction context is null for system compaction
+ if (runner.compaction != null) {
+ Collection<HStoreFile> files = runner.compaction.getRequest().getFiles();
Review comment:
Is this addressed?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
Do you want to resolve this comment? Looks like there is no sorting anymore.
--
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
[GitHub] [hbase] frostruan commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-850129971
@Apache9 @guangxuCheng hi, any questions here?
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-841398708
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 1m 2s | master passed |
| +1 :green_heart: | shadedjars | 8m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 44s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 171m 34s | hbase-server in the patch passed. |
| | | 202m 3s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux fd626aa6dd9f 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 0955a7a22e |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/testReport/ |
| Max. process+thread count | 4074 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] saintstack commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r633285717
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
Review comment:
Was this fixed then?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ // for system compaction, files selection will be delayed until the compaction task
+ // actually runs, so compaction context is null for system compaction
+ if (runner.compaction != null) {
+ Collection<HStoreFile> files = runner.compaction.getRequest().getFiles();
Review comment:
Is this addressed?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
Do you want to resolve this comment? Looks like there is no sorting anymore.
--
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
[GitHub] [hbase] Apache9 commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r637380447
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,11 +795,27 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
- longCompactions.getQueue().clear();
+ removeFromFilesCompacting(longCompactions);
}
public void clearShortCompactionsQueue() {
- shortCompactions.getQueue().clear();
+ removeFromFilesCompacting(shortCompactions);
+ }
+
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ Iterator<Runnable> iter = compactor.getQueue().iterator();
+ while (iter.hasNext()) {
+ Runnable runnable = iter.next();
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ if (runner.compaction != null && runner.compaction.hasSelection()) {
Review comment:
No data race here? Not sure, just asking
--
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
[GitHub] [hbase] guangxuCheng commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
guangxuCheng commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631685459
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ // for system compaction, files selection will be delayed until the compaction task
+ // actually runs, so compaction context is null for system compaction
+ if (runner.compaction != null) {
+ Collection<HStoreFile> files = runner.compaction.getRequest().getFiles();
Review comment:
Before obtaining the file list, you should check whether the compacting has already selected the files
--
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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631736048
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
I think we should always keep it sorted. some methods, like org.apache.hadoop.hbase.regionserver.compactions.SortedCompactionPolicy#getCurrentEligibleFiles(), may did not work if it is unsorted.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840355812
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 41s | master passed |
| +1 :green_heart: | compile | 3m 27s | master passed |
| +1 :green_heart: | checkstyle | 1m 4s | master passed |
| +1 :green_heart: | spotbugs | 2m 6s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 41s | the patch passed |
| +1 :green_heart: | compile | 3m 18s | the patch passed |
| +1 :green_heart: | javac | 3m 18s | the patch passed |
| -0 :warning: | checkstyle | 1m 5s | hbase-server: The patch generated 1 new + 6 unchanged - 0 fixed = 7 total (was 6) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 21s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 20s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 47m 51s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux abae082bf1ff 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b65733c093 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631736048
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
I think we should always keep it sorted. some methods, like org.apache.hadoop.hbase.regionserver.compactions.SortedCompactionPolicy#getCurrentEligibleFiles(), may will not work if it is unsorted.
--
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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631738868
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
Review comment:
yes. Instead of removing all files from file compacting list first and then clearing all compaction requests, maybe it is better to remove files and compaction request one by one. I will fix 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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840433555
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 36s | master passed |
| +1 :green_heart: | compile | 1m 15s | master passed |
| +1 :green_heart: | shadedjars | 10m 46s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 44s | the patch passed |
| +1 :green_heart: | compile | 1m 37s | the patch passed |
| +1 :green_heart: | javac | 1m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 11m 13s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 155m 32s | hbase-server in the patch passed. |
| | | 195m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux e1d5a6d1a259 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b65733c093 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/testReport/ |
| Max. process+thread count | 3752 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] frostruan closed pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan closed pull request #3261:
URL: https://github.com/apache/hbase/pull/3261
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-839959811
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 31s | master passed |
| +1 :green_heart: | compile | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 8m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 14s | the patch passed |
| +1 :green_heart: | compile | 1m 11s | the patch passed |
| +1 :green_heart: | javac | 1m 11s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 13s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 138m 34s | hbase-server in the patch passed. |
| | | 170m 8s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 16afd290c99c 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 630c73fda4 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/testReport/ |
| Max. process+thread count | 3995 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-839863654
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 20s | master passed |
| +1 :green_heart: | compile | 3m 19s | master passed |
| +1 :green_heart: | checkstyle | 1m 6s | master passed |
| +1 :green_heart: | spotbugs | 2m 13s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 40s | the patch passed |
| +1 :green_heart: | compile | 3m 16s | the patch passed |
| +1 :green_heart: | javac | 3m 16s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 1s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 17m 57s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 47m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 467a769b9cf0 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 630c73fda4 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] frostruan commented on pull request #3261: remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840083900
Thanks for taking a look on my PR.
For system compaction, selectNow will be false, which means files will be added to the compacting list when the compaction task is actually executed. So it is safe to just clear the workQueue of compaction thread pool executor.
However, for user-triggered compaction, selectNow will be true, which means files are already added to the compacting list when we put the compaction request to the queue. When CompactionPolicy selects candidate files for minor compaction, files in compacting list will be excluded. If we just clear the workQueue of compaction thread pool executor, files of these compaction requests are still in compacting list. These files will not be selected by CompactionPolicy and compacted any more unless we force a major compaction. So I think maybe we should remove those files from compacting list when we clear the compaction queue.
--
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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631736048
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
I think we should always keep it sorted. some methods, like org.apache.hadoop.hbase.regionserver.compactions.SortedCompactionPolicy#getCurrentEligibleFiles(), may will not work if it is unsorted.
--
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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631716344
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,23 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ CompactionRunner runner = (CompactionRunner) runnable;
Review comment:
I will add a type check here.
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-841302722
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 1s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 3m 22s | master passed |
| +1 :green_heart: | checkstyle | 1m 11s | master passed |
| +1 :green_heart: | spotbugs | 2m 12s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 0s | the patch passed |
| +1 :green_heart: | compile | 3m 19s | the patch passed |
| +1 :green_heart: | javac | 3m 19s | the patch passed |
| -0 :warning: | checkstyle | 1m 10s | hbase-server: The patch generated 1 new + 6 unchanged - 0 fixed = 7 total (was 6) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 0s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 51m 8s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux b87466df0187 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 0955a7a22e |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/4/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840540267
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 3m 37s | master passed |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | spotbugs | 2m 29s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 10s | the patch passed |
| +1 :green_heart: | compile | 3m 46s | the patch passed |
| +1 :green_heart: | javac | 3m 46s | the patch passed |
| -0 :warning: | checkstyle | 1m 10s | hbase-server: The patch generated 1 new + 6 unchanged - 0 fixed = 7 total (was 6) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 43s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 36s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 52m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 0983dceb87a3 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b65733c093 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-839967100
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | compile | 1m 2s | master passed |
| +1 :green_heart: | shadedjars | 8m 28s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | the patch passed |
| +1 :green_heart: | compile | 1m 2s | the patch passed |
| +1 :green_heart: | javac | 1m 2s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 31s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 148m 25s | hbase-server in the patch passed. |
| | | 179m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6f6eeb6c6be3 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 630c73fda4 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/testReport/ |
| Max. process+thread count | 4372 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r632560241
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
my bad. I'll fix it.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
yes. I have removed unnecessary sorting. Thanks for reviewing.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ // for system compaction, files selection will be delayed until the compaction task
+ // actually runs, so compaction context is null for system compaction
+ if (runner.compaction != null) {
+ Collection<HStoreFile> files = runner.compaction.getRequest().getFiles();
Review comment:
yes. I added a check to make sure compaction has selection. Thanks.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
Review comment:
I have fixed it, thanks.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
Review comment:
yes. This is done. Thanks.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,11 +795,27 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
- longCompactions.getQueue().clear();
+ removeFromFilesCompacting(longCompactions);
}
public void clearShortCompactionsQueue() {
- shortCompactions.getQueue().clear();
+ removeFromFilesCompacting(shortCompactions);
+ }
+
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ Iterator<Runnable> iter = compactor.getQueue().iterator();
+ while (iter.hasNext()) {
+ Runnable runnable = iter.next();
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ if (runner.compaction != null && runner.compaction.hasSelection()) {
Review comment:
Thanks for taking a look on this PR.
I don't think there is data race here. It's safe. Thanks.
--
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
[GitHub] [hbase] frostruan commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
frostruan commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631734192
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ if (!(runnable instanceof CompactionRunner)) {
+ continue;
+ }
+ CompactionRunner runner = (CompactionRunner) runnable;
+ // for system compaction, files selection will be delayed until the compaction task
+ // actually runs, so compaction context is null for system compaction
+ if (runner.compaction != null) {
+ Collection<HStoreFile> files = runner.compaction.getRequest().getFiles();
Review comment:
thanks for your advise. I will fix 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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840629942
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 33s | master passed |
| +1 :green_heart: | compile | 1m 12s | master passed |
| +1 :green_heart: | shadedjars | 8m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 44s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 13s | the patch passed |
| +1 :green_heart: | compile | 1m 15s | the patch passed |
| +1 :green_heart: | javac | 1m 15s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 31s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 152m 27s | hbase-server in the patch failed. |
| | | 184m 3s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 793af951d862 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b65733c093 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/testReport/ |
| Max. process+thread count | 3816 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840628159
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 0s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | shadedjars | 8m 3s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | the patch passed |
| +1 :green_heart: | compile | 1m 2s | the patch passed |
| +1 :green_heart: | javac | 1m 2s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 19s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 147m 51s | hbase-server in the patch failed. |
| | | 181m 9s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 54903e1883d9 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b65733c093 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/testReport/ |
| Max. process+thread count | 3992 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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
[GitHub] [hbase] guangxuCheng commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
guangxuCheng commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631686192
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
Review comment:
Why need to sort here？
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
Review comment:
If some requests fail to clean up the compacting files, it may cause some of the compacting files to be removed, but the compacting request has not been remove from the queue
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
Review comment:
ditto
--
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
[GitHub] [hbase] saintstack commented on a change in pull request #3261: remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r631319754
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,23 @@ void shutdownLongCompactions(){
}
public void clearLongCompactionsQueue() {
+ removeFromFilesCompacting(longCompactions);
longCompactions.getQueue().clear();
}
public void clearShortCompactionsQueue() {
+ removeFromFilesCompacting(shortCompactions);
shortCompactions.getQueue().clear();
}
+ private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+ for (Runnable runnable : compactor.getQueue()) {
+ CompactionRunner runner = (CompactionRunner) runnable;
Review comment:
For sure it will always be a CompactionRunner ? Should you check for it?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1953,22 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
}
+ /**
+ * Remove the files from compacting files. This usually happens when we clear compaction queues.
+ */
+ public void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+ synchronized (filesCompacting) {
+ filesCompacting.removeAll(filesToRemove);
+ Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+ }
+ }
+
+ public List<HStoreFile> getFilesCompacting() {
Review comment:
This is for test only?
See hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java for how methods like this are marked with a @RestrictedApi annotation.
It looks like this method and the one above it can be package-private rather than public?
--
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
[GitHub] [hbase] Apache-HBase commented on pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#issuecomment-840463870
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 4s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 20s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | shadedjars | 9m 18s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 34s | the patch passed |
| +1 :green_heart: | compile | 1m 9s | the patch passed |
| +1 :green_heart: | javac | 1m 9s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 18s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 219m 4s | hbase-server in the patch passed. |
| | | 253m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3261 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1219918df165 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b65733c093 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/testReport/ |
| Max. process+thread count | 3785 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3261/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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