Apache9 commented on a change in pull request #3202:
URL: https://github.com/apache/hbase/pull/3202#discussion_r625094672
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
##########
@@ -1684,6 +1684,9 @@
*/
public static final int BATCH_ROWS_THRESHOLD_DEFAULT = 5000;
+ public final static String COMPACTION_OFFLOAD_ENABLED =
"hbase.compaction.offload.enabled";
Review comment:
Does this need to be in HConstants?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
##########
@@ -3152,11 +3155,31 @@ private void getProcedureResult(long procId,
CompletableFuture<Void> future, int
return future;
}
- @Override
- public CompletableFuture<Map<ServerName, Boolean>> compactionSwitch(boolean
switchState,
- List<String> serverNamesList) {
+ private CompletableFuture<List<ServerName>> getMasterAndRegionServerList() {
+ CompletableFuture<List<ServerName>> future = new CompletableFuture<>();
+ addListener(getRegionServerList(new ArrayList<>()), (serverNames, err) -> {
+ if (err != null) {
+ future.completeExceptionally(err);
+ return;
+ }
+ List<ServerName> serverNameList = new ArrayList<>(serverNames);
+ addListener(getMaster(), (masterName, err1) -> {
+ if (err1 != null) {
+ future.completeExceptionally(err1);
+ } else {
+ serverNameList.add(masterName);
+ future.complete(serverNameList);
+ }
+ });
+ });
+ return future;
+ }
+
+ public CompletableFuture<Map<ServerName, Boolean>>
doSwitchForServers(boolean switchState,
Review comment:
Why public here?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
##########
@@ -542,6 +544,28 @@ boolean synchronousBalanceSwitch(final boolean b) throws
IOException {
return switchBalancer(b, BalanceSwitchMode.SYNC);
}
+ @Override
+ public CompactionOffloadSwitchResponse compactionOffloadSwitch(RpcController
controller,
+ CompactionOffloadSwitchRequest request) throws ServiceException {
+ // write compaction offload switch to zk
+ CompactionOffloadSwitchResponse.Builder response =
+ CompactionOffloadSwitchResponse.newBuilder();
+ try {
+ master.checkInitialized();
+ boolean newValue = request.getEnabled();
+ boolean oldValue =
master.isSplitOrMergeEnabled(MasterSwitchType.COMPACTION_OFFLOAD);
+ response.setPrevState(oldValue);
+ LOG.info("Set compaction offload enabled to {}", newValue);
+ master.getSplitOrMergeTracker().setSplitOrMergeEnabled(newValue,
Review comment:
Still called SplitOrMergeTracker?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -282,6 +284,9 @@
private HeapMemoryManager hMemManager;
+ private volatile boolean compactionOffloadEnabled = false;
+ private SplitOrMergeTracker switchTracker;
Review comment:
We do not use this field at region server side in the past? Then I think
we need to consider whether this is the correct approach.
I checked the design doc, not too much information about this. Could you
please explain the design more?
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:
[email protected]
