You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "mini666 (via GitHub)" <gi...@apache.org> on 2026/06/19 08:05:54 UTC
[PR] HBASE-30238 HBase bulkload replication causes duplicate HFile loading and compaction storm when source RPC times out [hbase]
mini666 opened a new pull request, #8380:
URL: https://github.com/apache/hbase/pull/8380
JIRA: https://issues.apache.org/jira/browse/HBASE-30238
This PR adds:
* Configurable bandwidth throttling for bulkloaded HFile copy during replication.
* In-progress bulkload deduplication for concurrent RPC retries of the same bulkload event.
Testing:
* `JAVA_HOME=/Users/juke.mini666/.asdf/installs/java/temurin-17.0.6+10 mvn test -pl hbase-server -am -Dtest=TestHFileReplicatorBandwidth,TestReplicationSinkBulkLoadDedup -DfailIfNoTests=false -Dhadoop-three.version=3.4.3 -Dwarbucks.skip=true -Dcheckstyle.skip=true -Dspotbugs.skip=true -Drat.skip=true`
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30238 HBase bulkload replication causes duplicate HFile loading and compaction storm when source RPC times out [hbase]
Posted by "mini666 (via GitHub)" <gi...@apache.org>.
mini666 commented on PR #8380:
URL: https://github.com/apache/hbase/pull/8380#issuecomment-4761296417
Thanks for the review.
I added a follow-up commit to handle the cross-target-RS retry case. The sink now coordinates replicated bulkload WAL events through ZooKeeper using in-progress and completed markers, so a retry sent to a different target region server can observe that the same bulkload event has already completed and skip reloading the HFiles.
I also added a master-side chore to clean expired completed markers, while keeping them if a matching in-progress marker still exists.
For coverage, I added unit tests for the ZooKeeper event tracker and cleanup chore, a sink-level completed-marker skip test, and a MiniCluster test that replays the exact same bulkload WAL batch first through one target RS sink and then through another target RS sink.
I verified the change with the focused bulkload replication tests, including the new cross-target-RS retry MiniCluster test. The run passed with 13 tests, 0 failures, 0 errors, and 0 skipped.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30238 HBase bulkload replication causes duplicate HFile loading and compaction storm when source RPC times out [hbase]
Posted by "wchevreuil (via GitHub)" <gi...@apache.org>.
wchevreuil commented on code in PR #8380:
URL: https://github.com/apache/hbase/pull/8380#discussion_r3498098671
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/ReplicationBulkLoadEventCleaner.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.master.cleaner;
+
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.Stoppable;
+import org.apache.hadoop.hbase.replication.regionserver.ReplicationBulkLoadEventTracker;
+import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.zookeeper.KeeperException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Cleans completed replicated bulk load event markers after they are old enough.
+ */
+@InterfaceAudience.Private
+public class ReplicationBulkLoadEventCleaner extends ScheduledChore {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ReplicationBulkLoadEventCleaner.class);
+
+ public static final String PERIOD_MS_KEY =
+ "hbase.master.cleaner.replication.bulkload.event.period.ms";
+ public static final int PERIOD_MS_DEFAULT = (int) TimeUnit.MINUTES.toMillis(10);
+ public static final String DONE_TTL_MS_KEY = "hbase.replication.bulkload.event.done.ttl.ms";
+ public static final long DONE_TTL_MS_DEFAULT = TimeUnit.DAYS.toMillis(1);
+
+ private final ReplicationBulkLoadEventTracker tracker;
+ private final long doneTtlMs;
+
+ public ReplicationBulkLoadEventCleaner(Configuration conf, Stoppable stopper, ZKWatcher zkw) {
+ super("ReplicationBulkLoadEventCleaner", stopper,
+ conf.getInt(PERIOD_MS_KEY, PERIOD_MS_DEFAULT));
+ this.tracker = new ReplicationBulkLoadEventTracker(conf, zkw);
+ this.doneTtlMs = conf.getLong(DONE_TTL_MS_KEY, DONE_TTL_MS_DEFAULT);
+ }
+
+ @Override
+ protected void chore() {
+ try {
+ int deleted = tracker.cleanDoneMarkers(doneTtlMs);
+ if (deleted > 0) {
+ LOG.info("Cleaned {} replicated bulkload event marker(s)", deleted);
+ }
+ } catch (KeeperException e) {
+ LOG.warn("Failed to clean replicated bulkload event markers", e);
+ }
+ }
+
+ public void choreForTesting() {
Review Comment:
There's no need for this extra method. Tests in the same package can simply call chore() directly. And the super class already defines same method, only difference being it's synchronized there.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationBulkLoadEventTracker.java:
##########
@@ -0,0 +1,268 @@
+/*
+ * 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.replication.regionserver;
+
+import java.io.IOException;
+import java.io.InterruptedIOException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.hbase.util.MD5Hash;
+import org.apache.hadoop.hbase.zookeeper.ZKUtil;
+import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
+import org.apache.hadoop.hbase.zookeeper.ZNodePaths;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Stat;
+
+/**
+ * Coordinates replicated bulk load events across all region servers in the sink cluster.
+ */
+@InterfaceAudience.Private
+public class ReplicationBulkLoadEventTracker {
Review Comment:
Having a tracker for this kind of events is a good idea, but we should not make it tight to ZK. There's been an effort to gradually move HBase features out of ZK. Replication queue storage, for example, already has a ZKless alternative (see HBASE-27109).
This tracker, should follow similar approach. We should have it as interface/abstract class and provide both ZK based and table based implementations. However, I don't think this should be implemented in this same PR. My suggestion for you then is to move this tracking logic out of this PR, let's leave this one simply handling the issue on individual RS level. Then once this is merged, you can file a new jira/PR to implement the tracking logic at a global level, covering both ZK and table based registries.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/ReplicationBulkLoadEventCleaner.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.master.cleaner;
+
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.Stoppable;
+import org.apache.hadoop.hbase.replication.regionserver.ReplicationBulkLoadEventTracker;
+import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.zookeeper.KeeperException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Cleans completed replicated bulk load event markers after they are old enough.
+ */
Review Comment:
Please add explanation about how the new configuration properties influence this chore behaviour.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30238 HBase bulkload replication causes duplicate HFile loading and compaction storm when source RPC times out [hbase]
Posted by "wchevreuil (via GitHub)" <gi...@apache.org>.
wchevreuil commented on code in PR #8380:
URL: https://github.com/apache/hbase/pull/8380#discussion_r3443186675
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java:
##########
@@ -422,6 +472,11 @@ private void addNewTableEntryInMap(
bulkLoadHFileMap.put(tableName, newFamilyHFilePathsList);
}
+ private static String buildBulkLoadKey(String replicationClusterId, BulkLoadDescriptor bld) {
Review Comment:
nit: no need to be private.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java:
##########
@@ -422,6 +472,11 @@ private void addNewTableEntryInMap(
bulkLoadHFileMap.put(tableName, newFamilyHFilePathsList);
}
+ private static String buildBulkLoadKey(String replicationClusterId, BulkLoadDescriptor bld) {
Review Comment:
nit: no need to be static.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30238 HBase bulkload replication causes duplicate HFile loading and compaction storm when source RPC times out [hbase]
Posted by "mini666 (via GitHub)" <gi...@apache.org>.
mini666 commented on PR #8380:
URL: https://github.com/apache/hbase/pull/8380#issuecomment-4988881493
I pushed an update addressing the review feedback.
This revision focuses on the correctness issue in this PR: preventing duplicate execution of replicated bulk-load events on retry/timeout paths. I split the event tracker into an interface and kept the current implementation backed by ZooKeeper. That keeps the sink-side lifecycle independent from the backend, while leaving a table-backed tracker as a possible follow-up without expanding this PR into a larger storage change.
The important behavior change is table-level completion. Events for a table are marked done as soon as that table is loaded successfully, and if a later table fails, already-loaded events are not released as retryable work. I also tightened the ZK claim path so it re-checks the done marker after creating the in-progress marker, and best-effort cleans up the marker this call created on the relevant post-create failure paths.
Local verification passed:
* Focused tests: 9 tests, 0 failures, 0 errors.
* Cross-RS duplicate bulk-load regression test: 1 test, 0 failures, 0 errors.
* `git diff --cached --check` before commit.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org