nyl3532016 commented on a change in pull request #3425:
URL: https://github.com/apache/hbase/pull/3425#discussion_r660495018
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CSRpcServices.java
##########
@@ -88,8 +93,18 @@ void start() {
public CompactResponse requestCompaction(RpcController controller,
CompactionProtos.CompactRequest request) {
requestCount.increment();
+ ServerName rsServerName = ProtobufUtil.toServerName(request.getServer());
+ RegionInfo regionInfo = ProtobufUtil.toRegionInfo(request.getRegionInfo());
+ ColumnFamilyDescriptor cfd =
ProtobufUtil.toColumnFamilyDescriptor(request.getFamily());
+ boolean major = request.getMajor();
+ int priority = request.getPriority();
+ List<HBaseProtos.ServerName> favoredNodes =
Collections.singletonList(request.getServer());
LOG.info("Receive compaction request from {}",
ProtobufUtil.toString(request));
- compactionServer.compactionThreadManager.requestCompaction();
+ CompactionTask compactionTask =
+
CompactionTask.newBuilder().setRsServerName(rsServerName).setRegionInfo(regionInfo)
+
.setColumnFamilyDescriptor(cfd).setRequestMajor(major).setPriority(priority)
+
.setFavoredNodes(favoredNodes).setSubmitTime(System.currentTimeMillis()).build();
+ compactionServer.compactionThreadManager.requestCompaction(compactionTask);
Review comment:
Yes, in our internal version, We have already implemented macro-level
throttling
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionTask.java
##########
@@ -0,0 +1,173 @@
+/**
+ * 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;
Review comment:
I only find `org.apache.hadoop.hbase.regionserver.compactions` package ?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionServerStorage.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+/**
+ * since we do not maintain StoreFileManager in compaction server(can't
refresh when flush). we use
+ * external storage(this class) to record compacting files, and initialize a
new HStore in
+ * {@link CompactionThreadManager#selectCompaction} every time when request
compaction
+ */
+class CompactionServerStorage {
+ private static Logger LOG =
LoggerFactory.getLogger(CompactionServerStorage.class);
+ private final ConcurrentMap<String, ConcurrentMap<String, Set<String>>>
selectedFiles =
+ new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, ConcurrentMap<String, Set<String>>>
compactedFiles =
+ new ConcurrentHashMap<>();
+ /**
+ * Mark files as completed, called after CS finished compaction and RS
accepted the results of
+ * this compaction, these compacted files will be deleted by RS if no reader
referenced to them.
+ */
+ boolean addCompactedFiles(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ List<String> compactedFiles) {
+ Set<String> compactedFileSet = getCompactedStoreFiles(regionInfo, cfd);
+ synchronized (compactedFileSet) {
+ compactedFileSet.addAll(compactedFiles);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Mark files as compacted, region: {}, cf, files: {}",
regionInfo,
+ cfd.getNameAsString(), compactedFileSet);
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Mark files as selected, called after the files are selected and before
the compaction is
+ * started. Avoid a file is selected twice in two compaction.
+ * @return True if these files don't be selected, false if these files are
already selected.
+ */
+ boolean addSelectedFiles(RegionInfo regionInfo, ColumnFamilyDescriptor cfd,
+ List<String> selectedFiles) {
+ Set<String> selectedFileSet = getSelectedStoreFiles(regionInfo, cfd);
+ synchronized (selectedFileSet) {
Review comment:
Yes, it is a local variable, but the non-primitive local variable refer
the object on heap, and synchronized block the heap object, I have added a test
`TestCompactionServerStorage#testSelectedFilesStoreMultiThread`
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/throttle/ThroughputControllerService.java
##########
@@ -0,0 +1,31 @@
+/**
+ * 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.regionserver.throttle;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ChoreService;
+import org.apache.yetus.audience.InterfaceAudience;
+
[email protected]
+public interface ThroughputControllerService {
+ ChoreService getChoreService();
+ double getCompactionPressure();
+ @Deprecated
+ double getFlushPressure();
Review comment:
Just do some format and abstract, keep original semantic.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactThreadControl.java
##########
@@ -0,0 +1,151 @@
+/**
+ * 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.regionserver;
+
+import
org.apache.hadoop.hbase.regionserver.throttle.CompactionThroughputControllerFactory;
+import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController;
+import
org.apache.hadoop.hbase.regionserver.throttle.ThroughputControllerService;
+import org.apache.hadoop.hbase.util.StealJobQueue;
+import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.RejectedExecutionHandler;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+
[email protected]
+public class CompactThreadControl {
Review comment:
Yes, the class `CompactThreadControl` is used by the CS. This class aim
to eliminate the duplicate code
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/throttle/ThroughputControllerService.java
##########
@@ -0,0 +1,31 @@
+/**
+ * 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.regionserver.throttle;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ChoreService;
+import org.apache.yetus.audience.InterfaceAudience;
+
[email protected]
+public interface ThroughputControllerService {
+ ChoreService getChoreService();
Review comment:
But this interface need this method
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/throttle/ThroughputControllerService.java
##########
@@ -0,0 +1,31 @@
+/**
+ * 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.regionserver.throttle;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ChoreService;
+import org.apache.yetus.audience.InterfaceAudience;
+
[email protected]
+public interface ThroughputControllerService {
Review comment:
OK
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionTask.java
##########
@@ -0,0 +1,173 @@
+/**
+ * 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.util.List;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.monitoring.MonitoredTask;
+import org.apache.hadoop.hbase.regionserver.HStore;
+import org.apache.hadoop.hbase.regionserver.compactions.CompactionContext;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
+
[email protected]
+public final class CompactionTask {
+ private ServerName rsServerName;
+ private RegionInfo regionInfo;
Review comment:
If region location move, ask master to get new location.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionServerStorage.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
[email protected]
+/**
+ * since we do not maintain StoreFileManager in compaction server(can't
refresh when flush). we use
+ * external storage(this class) to record compacting files, and initialize a
new HStore in
+ * {@link CompactionThreadManager#selectCompaction} every time when request
compaction
Review comment:
This class is not used in HRegionserver, only help CS to maintain
compacting and compacted files.
No, CS not do Hfile movement, when compaction finished, It call
`completeCompaction` against RS, then, RS do file movement
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CompactionTask.java
##########
@@ -0,0 +1,173 @@
+/**
+ * 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.util.List;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.monitoring.MonitoredTask;
+import org.apache.hadoop.hbase.regionserver.HStore;
+import org.apache.hadoop.hbase.regionserver.compactions.CompactionContext;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
+
[email protected]
+public final class CompactionTask {
Review comment:
We have `CompactionContext` in `CompactionTask`
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/compactionserver/CSRpcServices.java
##########
@@ -19,11 +19,15 @@
Review comment:
If CS could be in its own module, It should not depend on hbase-server.
So, the code of compaction should be extracted.I suppose this could be done at
a later stage
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
