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/06/14 07:29:12 UTC
[GitHub] [hbase] nyl3532016 opened a new pull request #3378: HBASE-25968 Request compact to compaction server
nyl3532016 opened a new pull request #3378:
URL: https://github.com/apache/hbase/pull/3378
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-862146614
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 8s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +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. |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 45s | HBASE-25714 passed |
| +1 :green_heart: | compile | 6m 50s | HBASE-25714 passed |
| +1 :green_heart: | checkstyle | 1m 55s | HBASE-25714 passed |
| +1 :green_heart: | spotbugs | 7m 21s | HBASE-25714 passed |
| -0 :warning: | patch | 2m 34s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 0s | the patch passed |
| +1 :green_heart: | compile | 5m 47s | the patch passed |
| +1 :green_heart: | cc | 5m 47s | the patch passed |
| +1 :green_heart: | javac | 5m 47s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 50s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 45s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | hbaseprotoc | 2m 3s | the patch passed |
| +1 :green_heart: | spotbugs | 7m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
| | | 74m 12s | |
| 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-3378/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux af7c6ed922e7 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/6/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] nyl3532016 commented on pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-860010181
> A general question, how to deal with rpc retry here? Is it safe to call requestCompaction many times with the same request?
Compaction request is idempotent and not that strict ? Repeated request rarely make an impact, for the compacting file can't be select and return an empty selected 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] Apache-HBase commented on pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-862222037
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 37s | HBASE-25714 passed |
| +1 :green_heart: | compile | 2m 12s | HBASE-25714 passed |
| +1 :green_heart: | shadedjars | 7m 46s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | HBASE-25714 passed |
| -0 :warning: | patch | 9m 33s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 35s | the patch passed |
| +1 :green_heart: | compile | 2m 14s | the patch passed |
| +1 :green_heart: | javac | 2m 14s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 39s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 47s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 1m 13s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 149m 53s | hbase-server in the patch passed. |
| | | 184m 59s | |
| 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-3378/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 39f019141322 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/6/testReport/ |
| Max. process+thread count | 4107 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/6/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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-862043669
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 25s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +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. |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 8s | HBASE-25714 passed |
| +1 :green_heart: | compile | 6m 47s | HBASE-25714 passed |
| +1 :green_heart: | checkstyle | 2m 16s | HBASE-25714 passed |
| +1 :green_heart: | spotbugs | 8m 42s | HBASE-25714 passed |
| -0 :warning: | patch | 3m 2s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 48s | the patch passed |
| +1 :green_heart: | compile | 6m 46s | the patch passed |
| +1 :green_heart: | cc | 6m 46s | the patch passed |
| +1 :green_heart: | javac | 6m 46s | the patch passed |
| -0 :warning: | checkstyle | 1m 32s | hbase-server: The patch generated 9 new + 103 unchanged - 0 fixed = 112 total (was 103) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 24m 27s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | hbaseprotoc | 2m 38s | the patch passed |
| +1 :green_heart: | spotbugs | 9m 28s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 33s | The patch does not generate ASF License warnings. |
| | | 86m 19s | |
| 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-3378/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux 5cef16b798a8 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/5/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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-862096283
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 38s | HBASE-25714 passed |
| +1 :green_heart: | compile | 2m 13s | HBASE-25714 passed |
| +1 :green_heart: | shadedjars | 7m 44s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | HBASE-25714 passed |
| -0 :warning: | patch | 9m 31s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 40s | the patch passed |
| +1 :green_heart: | compile | 2m 13s | the patch passed |
| +1 :green_heart: | javac | 2m 13s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 44s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 48s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 1m 12s | hbase-client in the patch passed. |
| -1 :x: | unit | 149m 59s | hbase-server in the patch failed. |
| | | 185m 23s | |
| 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-3378/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9aef09cf0943 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/5/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-3378/5/testReport/ |
| Max. process+thread count | 5102 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/5/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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r655042055
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -3762,6 +3766,40 @@ public ClearRegionBlockCacheResponse clearRegionBlockCache(RpcController control
return builder.setStats(ProtobufUtil.toCacheEvictionStats(stats.build())).build();
}
+
+ @Override
+ public CompleteCompactionResponse completeCompaction(RpcController controller,
+ CompleteCompactionRequest request) throws ServiceException {
+ RegionInfo regionInfo = ProtobufUtil.toRegionInfo(request.getRegionInfo());
+ ColumnFamilyDescriptor family = ProtobufUtil.toColumnFamilyDescriptor(request.getFamily());
+ LOG.debug("Region server receive complete compaction for region: {}, cf: {}",
+ regionInfo.getRegionNameAsString(), family.getNameAsString());
+ boolean success = false;
+ HRegion onlineRegion = regionServer.getOnlineRegion(regionInfo.getRegionName());
+ if (onlineRegion != null) {
+ HStore store = onlineRegion.getStore(family.getName());
+ if (store != null) {
+ if (store.getForceMajor()) {
+ store.setForceMajor(request.getNewForceMajor());
+ }
+ List<String> selectedFiles =
+ request.getSelectedFilesList().stream().collect(Collectors.toList());
+ List<String> newFiles = request.getNewFilesList().stream().collect(Collectors.toList());
+ try {
+ success = store.completeCompaction(null, selectedFiles, null, newFiles);
Review comment:
Recorded in TODO
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1966,6 +2021,34 @@ public boolean shouldPerformMajorCompaction() throws IOException {
return Optional.of(compaction);
}
+ private void preCompactionSelection(CompactionContext compaction,
Review comment:
yes, refactor code
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncCompactionServerService.java
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.client;
+
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback;
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcController;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactRequest;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
+
+
+/**
+ * A simple wrapper of the {@link CompactionService} for a compaction server, which returns a
+ * {@link CompletableFuture}. This is easier to use, as if you use the raw protobuf interface, you
+ * need to get the result from the {@link RpcCallback}, and if there is an exception, you need to
+ * get it from the {@link RpcController} passed in.
+ * <p/>
+ * Notice that there is no retry, and this is intentional. We have different retry for different
+ * usage for now, if later we want to unify them, we can move the retry logic into this class.
+ */
+@InterfaceAudience.Private
+public class AsyncCompactionServerService {
+
+ private final ServerName server;
+
+ private final AsyncClusterConnectionImpl conn;
+
+ AsyncCompactionServerService(ServerName server, AsyncClusterConnectionImpl conn) {
+ this.server = server;
+ this.conn = conn;
+ }
+
+ @FunctionalInterface
+ private interface RpcCall<RESP> {
+ void call(CompactionService.Interface stub, HBaseRpcController controller,
+ RpcCallback<RESP> done);
+ }
+
+ private <RESP> CompletableFuture<RESP> call(RpcCall<RESP> rpcCall) {
Review comment:
Recorded in TODO
--
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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r652327720
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
without master, we can't send compact request to compaction server. However, we can do compact in regionserver as original
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-862095896
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | 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 _ |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 9s | HBASE-25714 passed |
| +1 :green_heart: | compile | 2m 39s | HBASE-25714 passed |
| +1 :green_heart: | shadedjars | 7m 41s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 19s | HBASE-25714 passed |
| -0 :warning: | patch | 9m 35s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 17s | the patch passed |
| +1 :green_heart: | compile | 2m 42s | the patch passed |
| +1 :green_heart: | javac | 2m 42s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 46s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 20s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 0s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 1m 17s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 146m 12s | hbase-server in the patch passed. |
| | | 184m 44s | |
| 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-3378/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a1d22d043b8c 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/5/testReport/ |
| Max. process+thread count | 3935 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/5/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] Apache9 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r654932817
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1966,6 +2021,34 @@ public boolean shouldPerformMajorCompaction() throws IOException {
return Optional.of(compaction);
}
+ private void preCompactionSelection(CompactionContext compaction,
Review comment:
Just refactoring?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncCompactionServerService.java
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.client;
+
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback;
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcController;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactRequest;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
+
+
+/**
+ * A simple wrapper of the {@link CompactionService} for a compaction server, which returns a
+ * {@link CompletableFuture}. This is easier to use, as if you use the raw protobuf interface, you
+ * need to get the result from the {@link RpcCallback}, and if there is an exception, you need to
+ * get it from the {@link RpcController} passed in.
+ * <p/>
+ * Notice that there is no retry, and this is intentional. We have different retry for different
+ * usage for now, if later we want to unify them, we can move the retry logic into this class.
+ */
+@InterfaceAudience.Private
+public class AsyncCompactionServerService {
+
+ private final ServerName server;
+
+ private final AsyncClusterConnectionImpl conn;
+
+ AsyncCompactionServerService(ServerName server, AsyncClusterConnectionImpl conn) {
+ this.server = server;
+ this.conn = conn;
+ }
+
+ @FunctionalInterface
+ private interface RpcCall<RESP> {
+ void call(CompactionService.Interface stub, HBaseRpcController controller,
+ RpcCallback<RESP> done);
+ }
+
+ private <RESP> CompletableFuture<RESP> call(RpcCall<RESP> rpcCall) {
Review comment:
And this one is a call to master? If we want to follow the pattern here, maybe we could also change the way on how to do regionServerReport...
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -3762,6 +3766,40 @@ public ClearRegionBlockCacheResponse clearRegionBlockCache(RpcController control
return builder.setStats(ProtobufUtil.toCacheEvictionStats(stats.build())).build();
}
+
+ @Override
+ public CompleteCompactionResponse completeCompaction(RpcController controller,
+ CompleteCompactionRequest request) throws ServiceException {
+ RegionInfo regionInfo = ProtobufUtil.toRegionInfo(request.getRegionInfo());
+ ColumnFamilyDescriptor family = ProtobufUtil.toColumnFamilyDescriptor(request.getFamily());
+ LOG.debug("Region server receive complete compaction for region: {}, cf: {}",
+ regionInfo.getRegionNameAsString(), family.getNameAsString());
+ boolean success = false;
+ HRegion onlineRegion = regionServer.getOnlineRegion(regionInfo.getRegionName());
+ if (onlineRegion != null) {
+ HStore store = onlineRegion.getStore(family.getName());
+ if (store != null) {
+ if (store.getForceMajor()) {
+ store.setForceMajor(request.getNewForceMajor());
+ }
+ List<String> selectedFiles =
+ request.getSelectedFilesList().stream().collect(Collectors.toList());
+ List<String> newFiles = request.getNewFilesList().stream().collect(Collectors.toList());
+ try {
+ success = store.completeCompaction(null, selectedFiles, null, newFiles);
Review comment:
If we could write HFile directly into the data directory, here the completion will be easier. Just a note.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
So the logic is already implemented? I saw in the last catch block, we will return false, which is the same return value with compaction offload disabled.
This will be a challenge of the CP design, as for the same table, the compaction could be run in a compaction server or the region server randomly... Anyway, not the problem for this patch, but we need to consider again.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncCompactionServerService.java
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.client;
+
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback;
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcController;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactRequest;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
+
+
+/**
+ * A simple wrapper of the {@link CompactionService} for a compaction server, which returns a
+ * {@link CompletableFuture}. This is easier to use, as if you use the raw protobuf interface, you
+ * need to get the result from the {@link RpcCallback}, and if there is an exception, you need to
+ * get it from the {@link RpcController} passed in.
+ * <p/>
+ * Notice that there is no retry, and this is intentional. We have different retry for different
+ * usage for now, if later we want to unify them, we can move the retry logic into this class.
+ */
+@InterfaceAudience.Private
+public class AsyncCompactionServerService {
+
+ private final ServerName server;
+
+ private final AsyncClusterConnectionImpl conn;
+
+ AsyncCompactionServerService(ServerName server, AsyncClusterConnectionImpl conn) {
+ this.server = server;
+ this.conn = conn;
+ }
+
+ @FunctionalInterface
+ private interface RpcCall<RESP> {
+ void call(CompactionService.Interface stub, HBaseRpcController controller,
+ RpcCallback<RESP> done);
+ }
+
+ private <RESP> CompletableFuture<RESP> call(RpcCall<RESP> rpcCall) {
Review comment:
Oh, the one in ConnectionUtils is not suitable here as it requires ClientService.Interface. But we do have another version in AsyncRegionServerAdmin which has almost the same code. Should find a way to share the code in the future.
--
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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r652321614
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncCompactionServerService.java
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.client;
+
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback;
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcController;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactRequest;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
+
+
+/**
+ * A simple wrapper of the {@link CompactionService} for a compaction server, which returns a
+ * {@link CompletableFuture}. This is easier to use, as if you use the raw protobuf interface, you
+ * need to get the result from the {@link RpcCallback}, and if there is an exception, you need to
+ * get it from the {@link RpcController} passed in.
+ * <p/>
+ * Notice that there is no retry, and this is intentional. We have different retry for different
+ * usage for now, if later we want to unify them, we can move the retry logic into this class.
+ */
+@InterfaceAudience.Private
+public class AsyncCompactionServerService {
+
+ private final ServerName server;
+
+ private final AsyncClusterConnectionImpl conn;
+
+ AsyncCompactionServerService(ServerName server, AsyncClusterConnectionImpl conn) {
+ this.server = server;
+ this.conn = conn;
+ }
+
+ @FunctionalInterface
+ private interface RpcCall<RESP> {
+ void call(CompactionService.Interface stub, HBaseRpcController controller,
+ RpcCallback<RESP> done);
+ }
+
+ private <RESP> CompletableFuture<RESP> call(RpcCall<RESP> rpcCall) {
Review comment:
yes, the `call` method in `ConnectionUtils` agaist `ClientService.Interface`
--
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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r652325253
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncClusterConnectionImpl.java
##########
@@ -41,20 +45,36 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.CleanupBulkLoadResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.PrepareBulkLoadRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.PrepareBulkLoadResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier.RegionSpecifierType;
+import static org.apache.hadoop.hbase.client.ConnectionUtils.getStubKey;
/**
* The implementation of AsyncClusterConnection.
*/
@InterfaceAudience.Private
class AsyncClusterConnectionImpl extends AsyncConnectionImpl implements AsyncClusterConnection {
-
+ private final ConcurrentMap<String, CompactionService.Interface> CompactionSubs = new ConcurrentHashMap<>();
public AsyncClusterConnectionImpl(Configuration conf, ConnectionRegistry registry,
String clusterId, SocketAddress localAddress, User user) {
super(conf, registry, clusterId, localAddress, user);
}
+ CompactionProtos.CompactionService.Interface getCompactionStub(ServerName serverName) throws
Review comment:
remove the cache first
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-859469486
--
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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r652294366
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
##########
@@ -100,7 +100,8 @@
private final NonceGenerator nonceGenerator;
private final ConcurrentMap<String, ClientService.Interface> rsStubs = new ConcurrentHashMap<>();
- private final ConcurrentMap<String, AdminService.Interface> adminSubs = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, AdminService.Interface> adminStubs =
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-861220301
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 5s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 21s | HBASE-25714 passed |
| +1 :green_heart: | compile | 2m 46s | HBASE-25714 passed |
| +1 :green_heart: | shadedjars | 7m 46s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 23s | HBASE-25714 passed |
| -0 :warning: | patch | 9m 43s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 14s | the patch passed |
| +1 :green_heart: | compile | 2m 40s | the patch passed |
| +1 :green_heart: | javac | 2m 40s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 59s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 1m 18s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 147m 25s | hbase-server in the patch passed. |
| | | 186m 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-3378/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux edc5c55f2c6e 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/4/testReport/ |
| Max. process+thread count | 3835 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/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] Apache9 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r650031294
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
##########
@@ -101,6 +102,7 @@
private final ConcurrentMap<String, ClientService.Interface> rsStubs = new ConcurrentHashMap<>();
private final ConcurrentMap<String, AdminService.Interface> adminSubs = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, CompactionService.Interface> CompactionSubs = new ConcurrentHashMap<>();
Review comment:
Is this the suitable place? I do not think we want to expose this to client?
nit: the first letter should be lowercase, and it is Stubs, not Subs. The adminSubs should be a typo...
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionThreadManager.java
##########
@@ -0,0 +1,61 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.compactionserver;
+
+import java.io.IOException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.AsyncRegionServerAdmin;
+import org.apache.hadoop.hbase.trace.TraceUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class CompactionThreadManager {
+ private static Logger LOG = LoggerFactory.getLogger(CompactionThreadManager.class);
+
+ // process name
+ private final Configuration conf;
+ private final ConcurrentMap<ServerName, AsyncRegionServerAdmin> rsAdmins = new ConcurrentHashMap<>();
Review comment:
OK, now we will call back to region server to finish the compaction...
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionThreadManager.java
##########
@@ -0,0 +1,61 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.compactionserver;
+
+import java.io.IOException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.AsyncRegionServerAdmin;
+import org.apache.hadoop.hbase.trace.TraceUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class CompactionThreadManager {
+ private static Logger LOG = LoggerFactory.getLogger(CompactionThreadManager.class);
+
+ // process name
+ private final Configuration conf;
+ private final ConcurrentMap<ServerName, AsyncRegionServerAdmin> rsAdmins = new ConcurrentHashMap<>();
+ private final HCompactionServer server;
+
+ public CompactionThreadManager(final Configuration conf, HCompactionServer server) {
+ TraceUtil.initTracer(conf);
+ this.conf = conf;
+ this.server = server;
+ }
+
+ private AsyncRegionServerAdmin getRsAdmin(final ServerName sn) throws IOException {
+ AsyncRegionServerAdmin admin = this.rsAdmins.get(sn);
+ if (admin == null) {
+ LOG.debug("New RS admin connection to {}", sn);
+ admin = this.server.getAsyncClusterConnection().getRegionServerAdmin(sn);
+ this.rsAdmins.put(sn, admin);
+ }
+ return admin;
+ }
+
+ public void requestCompaction() {
Review comment:
Not implemented?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncClusterConnection.java
##########
@@ -41,6 +41,11 @@
*/
AsyncRegionServerAdmin getRegionServerAdmin(ServerName serverName);
+ /**
+ * Get the compaction service for the given compaction server.
+ */
+ AsyncCompactionServerCaller getCompactionServerCaller(ServerName serverName);
Review comment:
Why call it caller?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
OK, so we will call to master first. What is the purpose here? Better throttle?
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-861167250
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +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. |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 50s | HBASE-25714 passed |
| +1 :green_heart: | compile | 6m 46s | HBASE-25714 passed |
| +1 :green_heart: | checkstyle | 2m 13s | HBASE-25714 passed |
| +1 :green_heart: | spotbugs | 8m 59s | HBASE-25714 passed |
| -0 :warning: | patch | 3m 4s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 30s | the patch passed |
| +1 :green_heart: | compile | 6m 46s | the patch passed |
| +1 :green_heart: | cc | 6m 46s | the patch passed |
| +1 :green_heart: | javac | 6m 46s | the patch passed |
| -0 :warning: | checkstyle | 1m 21s | 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: | hadoopcheck | 19m 53s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | hbaseprotoc | 2m 5s | the patch passed |
| +1 :green_heart: | spotbugs | 8m 8s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 32s | The patch does not generate ASF License warnings. |
| | | 78m 4s | |
| 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-3378/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile cc hbaseprotoc prototool |
| uname | Linux 1f46c3d8cb88 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-862223592
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 8s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 8s | HBASE-25714 passed |
| +1 :green_heart: | compile | 2m 42s | HBASE-25714 passed |
| +1 :green_heart: | shadedjars | 7m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 21s | HBASE-25714 passed |
| -0 :warning: | patch | 9m 41s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 12s | the patch passed |
| +1 :green_heart: | compile | 2m 41s | the patch passed |
| +1 :green_heart: | javac | 2m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 0s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 1m 16s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 149m 1s | hbase-server in the patch passed. |
| | | 187m 27s | |
| 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-3378/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux abdcfa1b3776 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/6/testReport/ |
| Max. process+thread count | 3705 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/6/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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r650335399
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
Yes, we can global control the throttle, the compaction task to compaction server mapping and so on
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionThreadManager.java
##########
@@ -0,0 +1,61 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.compactionserver;
+
+import java.io.IOException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.AsyncRegionServerAdmin;
+import org.apache.hadoop.hbase.trace.TraceUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class CompactionThreadManager {
+ private static Logger LOG = LoggerFactory.getLogger(CompactionThreadManager.class);
+
+ // process name
+ private final Configuration conf;
+ private final ConcurrentMap<ServerName, AsyncRegionServerAdmin> rsAdmins = new ConcurrentHashMap<>();
+ private final HCompactionServer server;
+
+ public CompactionThreadManager(final Configuration conf, HCompactionServer server) {
+ TraceUtil.initTracer(conf);
+ this.conf = conf;
+ this.server = server;
+ }
+
+ private AsyncRegionServerAdmin getRsAdmin(final ServerName sn) throws IOException {
+ AsyncRegionServerAdmin admin = this.rsAdmins.get(sn);
+ if (admin == null) {
+ LOG.debug("New RS admin connection to {}", sn);
+ admin = this.server.getAsyncClusterConnection().getRegionServerAdmin(sn);
+ this.rsAdmins.put(sn, admin);
+ }
+ return admin;
+ }
+
+ public void requestCompaction() {
Review comment:
Yes, this implement left to next patch, HBASE-25991
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
##########
@@ -101,6 +102,7 @@
private final ConcurrentMap<String, ClientService.Interface> rsStubs = new ConcurrentHashMap<>();
private final ConcurrentMap<String, AdminService.Interface> adminSubs = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, CompactionService.Interface> CompactionSubs = new ConcurrentHashMap<>();
Review comment:
move the code to `AsyncClusterConnectionImpl` first
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncClusterConnection.java
##########
@@ -41,6 +41,11 @@
*/
AsyncRegionServerAdmin getRegionServerAdmin(ServerName serverName);
+ /**
+ * Get the compaction service for the given compaction server.
+ */
+ AsyncCompactionServerCaller getCompactionServerCaller(ServerName serverName);
Review comment:
not caller, I will amend it
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionThreadManager.java
##########
@@ -0,0 +1,61 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.compactionserver;
+
+import java.io.IOException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.AsyncRegionServerAdmin;
+import org.apache.hadoop.hbase.trace.TraceUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class CompactionThreadManager {
+ private static Logger LOG = LoggerFactory.getLogger(CompactionThreadManager.class);
+
+ // process name
+ private final Configuration conf;
+ private final ConcurrentMap<ServerName, AsyncRegionServerAdmin> rsAdmins = new ConcurrentHashMap<>();
Review comment:
Yes, call back to regionserver directly (not through master) may be more simple and effective
--
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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r652305875
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
without master, we can't send compact request to compaction server. However, we can do compact in regionserver as original
--
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] nyl3532016 merged pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 merged pull request #3378:
URL: https://github.com/apache/hbase/pull/3378
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r651758562
##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
##########
@@ -100,7 +100,8 @@
private final NonceGenerator nonceGenerator;
private final ConcurrentMap<String, ClientService.Interface> rsStubs = new ConcurrentHashMap<>();
- private final ConcurrentMap<String, AdminService.Interface> adminSubs = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, AdminService.Interface> adminStubs =
Review comment:
I think this could be a separated issue which target master branch?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncClusterConnectionImpl.java
##########
@@ -41,20 +45,36 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.CleanupBulkLoadResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.PrepareBulkLoadRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.PrepareBulkLoadResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier.RegionSpecifierType;
+import static org.apache.hadoop.hbase.client.ConnectionUtils.getStubKey;
/**
* The implementation of AsyncClusterConnection.
*/
@InterfaceAudience.Private
class AsyncClusterConnectionImpl extends AsyncConnectionImpl implements AsyncClusterConnection {
-
+ private final ConcurrentMap<String, CompactionService.Interface> CompactionSubs = new ConcurrentHashMap<>();
Review comment:
Stubs
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncClusterConnectionImpl.java
##########
@@ -41,20 +45,36 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.CleanupBulkLoadResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.PrepareBulkLoadRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.PrepareBulkLoadResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.RegionSpecifier.RegionSpecifierType;
+import static org.apache.hadoop.hbase.client.ConnectionUtils.getStubKey;
/**
* The implementation of AsyncClusterConnection.
*/
@InterfaceAudience.Private
class AsyncClusterConnectionImpl extends AsyncConnectionImpl implements AsyncClusterConnection {
-
+ private final ConcurrentMap<String, CompactionService.Interface> CompactionSubs = new ConcurrentHashMap<>();
public AsyncClusterConnectionImpl(Configuration conf, ConnectionRegistry registry,
String clusterId, SocketAddress localAddress, User user) {
super(conf, registry, clusterId, localAddress, user);
}
+ CompactionProtos.CompactionService.Interface getCompactionStub(ServerName serverName) throws
Review comment:
Do we really need to cache the stub here? This is not the normal read/write path, just create a new one everytime is enough? The rpc connection is cached in RpcClient.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
So without master, we can not reassign region, and now, we can not even compact a region...
Could we double check if this is really the only choice here?
Maybe start a discuss thread on dev list?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/client/AsyncCompactionServerService.java
##########
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.client;
+
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcCallback;
+import org.apache.hbase.thirdparty.com.google.protobuf.RpcController;
+
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactRequest;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.CompactionProtos.CompactionService;
+
+
+/**
+ * A simple wrapper of the {@link CompactionService} for a compaction server, which returns a
+ * {@link CompletableFuture}. This is easier to use, as if you use the raw protobuf interface, you
+ * need to get the result from the {@link RpcCallback}, and if there is an exception, you need to
+ * get it from the {@link RpcController} passed in.
+ * <p/>
+ * Notice that there is no retry, and this is intentional. We have different retry for different
+ * usage for now, if later we want to unify them, we can move the retry logic into this class.
+ */
+@InterfaceAudience.Private
+public class AsyncCompactionServerService {
+
+ private final ServerName server;
+
+ private final AsyncClusterConnectionImpl conn;
+
+ AsyncCompactionServerService(ServerName server, AsyncClusterConnectionImpl conn) {
+ this.server = server;
+ this.conn = conn;
+ }
+
+ @FunctionalInterface
+ private interface RpcCall<RESP> {
+ void call(CompactionService.Interface stub, HBaseRpcController controller,
+ RpcCallback<RESP> done);
+ }
+
+ private <RESP> CompletableFuture<RESP> call(RpcCall<RESP> rpcCall) {
Review comment:
We do not have a common method in ConnectionUtils?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionThreadManager.java
##########
@@ -0,0 +1,61 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.compactionserver;
+
+import java.io.IOException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.AsyncRegionServerAdmin;
+import org.apache.hadoop.hbase.trace.TraceUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class CompactionThreadManager {
+ private static Logger LOG = LoggerFactory.getLogger(CompactionThreadManager.class);
+
+ private final Configuration conf;
+ private final ConcurrentMap<ServerName, AsyncRegionServerAdmin> rsAdmins =
+ new ConcurrentHashMap<>();
+ private final HCompactionServer server;
+
+ public CompactionThreadManager(final Configuration conf, HCompactionServer server) {
+ TraceUtil.initTracer(conf);
Review comment:
What's this...
--
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] nyl3532016 commented on a change in pull request #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
nyl3532016 commented on a change in pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#discussion_r652305875
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -3736,4 +3741,67 @@ public CompactedHFilesDischarger getCompactedHFilesDischarger() {
public long getRetryPauseTime() {
return this.retryPauseTime;
}
+
+ @Override
+ public boolean isCompactionOffloadEnabled(){
+ return regionServerCompactionOffloadManager.isCompactionOffloadEnabled();
+ }
+
+ private synchronized void createCompactionManagerStub(boolean refresh) {
+ // Create Master Compaction service stub without refreshing the master node from ZK,
+ // use cached data
+ if (cmsStub == null) {
+ cmsStub =
+ (CompactionService.BlockingInterface) createMasterStub(CompactionService.class, refresh);
+ }
+ }
+
+ /**
+ * Send compaction request to compaction manager
+ * @return True if send request successfully, otherwise false
+ * @throws IOException If an error occurs
+ */
+ @Override
+ public boolean requestCompactRegion(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ boolean major, int priority) {
+ if (!isCompactionOffloadEnabled()) {
+ return false;
+ }
+ if (cmsStub == null) {
+ createCompactionManagerStub(false);
Review comment:
without master, we can't send compact request to compaction server. However, we can do compact in regionserver as original
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-864688672
--
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 #3378: HBASE-25968 Request compact to compaction server
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3378:
URL: https://github.com/apache/hbase/pull/3378#issuecomment-861219543
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ HBASE-25714 Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 39s | HBASE-25714 passed |
| +1 :green_heart: | compile | 2m 12s | HBASE-25714 passed |
| +1 :green_heart: | shadedjars | 7m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | HBASE-25714 passed |
| -0 :warning: | patch | 9m 33s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 37s | the patch passed |
| +1 :green_heart: | compile | 2m 12s | the patch passed |
| +1 :green_heart: | javac | 2m 12s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 44s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 46s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 1m 13s | hbase-client in the patch passed. |
| +1 :green_heart: | unit | 149m 50s | hbase-server in the patch passed. |
| | | 185m 15s | |
| 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-3378/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3378 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 12767b5217a9 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 | HBASE-25714 / 6afca943ea |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/4/testReport/ |
| Max. process+thread count | 4319 (vs. ulimit of 30000) |
| modules | C: hbase-protocol-shaded hbase-client hbase-server U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3378/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