Skip to content

Commit 91c480f

Browse files
committed
HBASE-30238 Prevent duplicate bulkload replication on RPC retry
Track in-progress bulkload events by replication cluster, encoded region, and bulkload sequence number so concurrent RPC retries skip duplicate execution. The key is removed after the current attempt completes or fails, preserving at-least-once retry semantics while avoiding concurrent duplicate loads.
1 parent 0e68185 commit 91c480f

2 files changed

Lines changed: 246 additions & 12 deletions

File tree

hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,15 @@
3535
import java.util.List;
3636
import java.util.Map;
3737
import java.util.Map.Entry;
38+
import java.util.Set;
3839
import java.util.TreeMap;
3940
import java.util.UUID;
41+
import java.util.concurrent.ConcurrentHashMap;
4042
import java.util.concurrent.Future;
4143
import java.util.concurrent.atomic.AtomicLong;
4244
import java.util.stream.Collectors;
4345
import org.apache.commons.lang3.StringUtils;
4446
import org.apache.hadoop.conf.Configuration;
45-
import org.apache.hadoop.hbase.conf.ConfigurationObserver;
4647
import org.apache.hadoop.fs.Path;
4748
import org.apache.hadoop.hbase.Cell;
4849
import org.apache.hadoop.hbase.CellUtil;
@@ -60,6 +61,7 @@
6061
import org.apache.hadoop.hbase.client.Put;
6162
import org.apache.hadoop.hbase.client.RetriesExhaustedException;
6263
import org.apache.hadoop.hbase.client.Row;
64+
import org.apache.hadoop.hbase.conf.ConfigurationObserver;
6365
import org.apache.hadoop.hbase.regionserver.RegionServerCoprocessorHost;
6466
import org.apache.hadoop.hbase.replication.ReplicationUtils;
6567
import org.apache.hadoop.hbase.security.UserProvider;
@@ -111,6 +113,9 @@ public class ReplicationSink implements ConfigurationObserver {
111113
private SourceFSConfigurationProvider provider;
112114
private WALEntrySinkFilter walEntrySinkFilter;
113115
private final RateLimiter bulkLoadCopyRateLimiter = RateLimiter.create(Double.MAX_VALUE);
116+
// Tracks bulkload events currently being processed to prevent duplicate execution on RPC retry.
117+
// Key: replicationClusterId + "#" + encodedRegionName + "#" + bulkloadSeqNum
118+
private final Set<String> inProgressBulkLoads = ConcurrentHashMap.newKeySet();
114119

115120
/**
116121
* Row size threshold for multi requests above which a warning is logged
@@ -158,6 +163,10 @@ public void onConfigurationChange(Configuration newConf) {
158163
return bulkLoadCopyRateLimiter.getRate();
159164
}
160165

166+
Set<String> getInProgressBulkLoads() {
167+
return inProgressBulkLoads;
168+
}
169+
161170
private void updateBulkLoadCopyBandwidth(Configuration conf) {
162171
double bandwidthMb = conf.getDouble(HFileReplicator.REPLICATION_BULKLOAD_COPY_BANDWIDTH_MB_KEY,
163172
HFileReplicator.REPLICATION_BULKLOAD_COPY_BANDWIDTH_MB_DEFAULT);
@@ -230,6 +239,8 @@ public void replicateEntries(List<WALEntry> entries, final ExtendedCellScanner c
230239
Map<TableName, Map<List<UUID>, List<Row>>> rowMap = new TreeMap<>();
231240

232241
Map<List<String>, Map<String, List<Pair<byte[], List<String>>>>> bulkLoadsPerClusters = null;
242+
// bulkload keys registered in inProgressBulkLoads for this batch, to be removed on completion
243+
List<String> registeredBulkLoadKeys = null;
233244
Pair<List<Mutation>, List<WALEntry>> mutationsToWalEntriesPairs =
234245
new Pair<>(new ArrayList<>(), new ArrayList<>());
235246
for (WALEntry entry : entries) {
@@ -262,6 +273,16 @@ public void replicateEntries(List<WALEntry> entries, final ExtendedCellScanner c
262273
if (CellUtil.matchingQualifier(cell, WALEdit.BULK_LOAD)) {
263274
BulkLoadDescriptor bld = WALEdit.getBulkLoadDescriptor(cell);
264275
if (bld.getReplicate()) {
276+
String bulkLoadKey = buildBulkLoadKey(replicationClusterId, bld);
277+
if (!inProgressBulkLoads.add(bulkLoadKey)) {
278+
LOG.warn("Skipping duplicate bulkload replication, already in progress: {}",
279+
bulkLoadKey);
280+
continue;
281+
}
282+
if (registeredBulkLoadKeys == null) {
283+
registeredBulkLoadKeys = new ArrayList<>();
284+
}
285+
registeredBulkLoadKeys.add(bulkLoadKey);
265286
if (bulkLoadsPerClusters == null) {
266287
bulkLoadsPerClusters = new HashMap<>();
267288
}
@@ -333,19 +354,26 @@ public void replicateEntries(List<WALEntry> entries, final ExtendedCellScanner c
333354
}
334355

335356
if (bulkLoadsPerClusters != null) {
336-
for (Entry<List<String>,
337-
Map<String, List<Pair<byte[], List<String>>>>> entry : bulkLoadsPerClusters.entrySet()) {
338-
Map<String, List<Pair<byte[], List<String>>>> bulkLoadHFileMap = entry.getValue();
339-
if (bulkLoadHFileMap != null && !bulkLoadHFileMap.isEmpty()) {
340-
LOG.debug("Replicating {} bulk loaded data", entry.getKey().toString());
341-
Configuration providerConf = this.provider.getConf(this.conf, replicationClusterId);
342-
try (HFileReplicator hFileReplicator = new HFileReplicator(providerConf,
343-
sourceBaseNamespaceDirPath, sourceHFileArchiveDirPath, bulkLoadHFileMap, conf,
344-
getConnection(), entry.getKey(), bulkLoadCopyRateLimiter)) {
345-
hFileReplicator.replicate();
346-
LOG.debug("Finished replicating {} bulk loaded data", entry.getKey().toString());
357+
try {
358+
for (Entry<List<String>,
359+
Map<String, List<Pair<byte[], List<String>>>>> entry : bulkLoadsPerClusters
360+
.entrySet()) {
361+
Map<String, List<Pair<byte[], List<String>>>> bulkLoadHFileMap = entry.getValue();
362+
if (bulkLoadHFileMap != null && !bulkLoadHFileMap.isEmpty()) {
363+
LOG.debug("Replicating {} bulk loaded data", entry.getKey().toString());
364+
Configuration providerConf = this.provider.getConf(this.conf, replicationClusterId);
365+
try (HFileReplicator hFileReplicator = new HFileReplicator(providerConf,
366+
sourceBaseNamespaceDirPath, sourceHFileArchiveDirPath, bulkLoadHFileMap, conf,
367+
getConnection(), entry.getKey(), bulkLoadCopyRateLimiter)) {
368+
hFileReplicator.replicate();
369+
LOG.debug("Finished replicating {} bulk loaded data", entry.getKey().toString());
370+
}
347371
}
348372
}
373+
} finally {
374+
if (registeredBulkLoadKeys != null) {
375+
inProgressBulkLoads.removeAll(registeredBulkLoadKeys);
376+
}
349377
}
350378
}
351379

@@ -444,6 +472,11 @@ private void addNewTableEntryInMap(
444472
bulkLoadHFileMap.put(tableName, newFamilyHFilePathsList);
445473
}
446474

475+
private static String buildBulkLoadKey(String replicationClusterId, BulkLoadDescriptor bld) {
476+
return replicationClusterId + "#" + Bytes.toString(bld.getEncodedRegionName().toByteArray())
477+
+ "#" + bld.getBulkloadSeqNum();
478+
}
479+
447480
private String getHFilePath(TableName table, BulkLoadDescriptor bld, String storeFile,
448481
byte[] family) {
449482
return new StringBuilder(100).append(table.getNamespaceAsString()).append(Path.SEPARATOR)
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.replication.regionserver;
19+
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertFalse;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
24+
import java.util.Collections;
25+
import java.util.List;
26+
import org.apache.hadoop.conf.Configuration;
27+
import org.apache.hadoop.hbase.HBaseTestingUtil;
28+
import org.apache.hadoop.hbase.HConstants;
29+
import org.apache.hadoop.hbase.PrivateCellUtil;
30+
import org.apache.hadoop.hbase.TableName;
31+
import org.apache.hadoop.hbase.testclassification.ReplicationTests;
32+
import org.apache.hadoop.hbase.testclassification.SmallTests;
33+
import org.apache.hadoop.hbase.util.Bytes;
34+
import org.apache.hadoop.hbase.wal.WALEdit;
35+
import org.apache.hadoop.hbase.wal.WALEditInternalHelper;
36+
import org.junit.jupiter.api.AfterAll;
37+
import org.junit.jupiter.api.BeforeAll;
38+
import org.junit.jupiter.api.Tag;
39+
import org.junit.jupiter.api.Test;
40+
import org.junit.jupiter.api.TestInstance;
41+
42+
import org.apache.hbase.thirdparty.com.google.protobuf.UnsafeByteOperations;
43+
44+
import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
45+
import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.WALEntry;
46+
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.UUID;
47+
import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos;
48+
import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos.WALKey;
49+
50+
/**
51+
* Unit tests for bulkload deduplication in {@link ReplicationSink}. Verifies that concurrent RPC
52+
* retries for the same bulkload event do not result in duplicate processing.
53+
*/
54+
@Tag(ReplicationTests.TAG)
55+
@Tag(SmallTests.TAG)
56+
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
57+
public class TestReplicationSinkBulkLoadDedup {
58+
59+
private static final String CLUSTER_ID_A = "cluster-A";
60+
private static final String CLUSTER_ID_B = "cluster-B";
61+
private static final byte[] REGION_NAME = Bytes.toBytes("regionXYZ");
62+
private static final long SEQ_NUM = 100L;
63+
private static final TableName TABLE = TableName.valueOf("testDedup");
64+
private static final byte[] FAMILY = Bytes.toBytes("cf");
65+
66+
private final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
67+
private ReplicationSink sink;
68+
69+
@BeforeAll
70+
public void setUpBeforeClass() throws Exception {
71+
Configuration conf = TEST_UTIL.getConfiguration();
72+
conf.set("hbase.replication.source.fs.conf.provider",
73+
TestSourceFSConfigurationProvider.class.getCanonicalName());
74+
sink = new ReplicationSink(conf, null);
75+
}
76+
77+
@AfterAll
78+
public void tearDownAfterClass() {
79+
// no cluster to shut down
80+
}
81+
82+
/**
83+
* Verify that manually adding a key to inProgressBulkLoads blocks a second add for the same key,
84+
* and that remove restores it — core Set semantics that the dedup logic relies on.
85+
*/
86+
@Test
87+
public void testInProgressSetAddAndRemove() {
88+
String key = CLUSTER_ID_A + "#" + Bytes.toString(REGION_NAME) + "#" + SEQ_NUM;
89+
90+
assertTrue(sink.getInProgressBulkLoads().add(key), "First add should succeed");
91+
assertFalse(sink.getInProgressBulkLoads().add(key),
92+
"Second add should fail (already in progress)");
93+
94+
sink.getInProgressBulkLoads().remove(key);
95+
assertTrue(sink.getInProgressBulkLoads().add(key),
96+
"Add after remove should succeed (retry allowed)");
97+
sink.getInProgressBulkLoads().remove(key); // cleanup
98+
}
99+
100+
/**
101+
* Verify that keys from different source clusters with the same region/seqNum are treated as
102+
* distinct entries and do not block each other.
103+
*/
104+
@Test
105+
public void testDifferentClustersDontConflict() {
106+
String keyA = CLUSTER_ID_A + "#" + Bytes.toString(REGION_NAME) + "#" + SEQ_NUM;
107+
String keyB = CLUSTER_ID_B + "#" + Bytes.toString(REGION_NAME) + "#" + SEQ_NUM;
108+
109+
assertTrue(sink.getInProgressBulkLoads().add(keyA));
110+
assertTrue(sink.getInProgressBulkLoads().add(keyB),
111+
"Same region/seqNum from different cluster should not conflict");
112+
113+
sink.getInProgressBulkLoads().remove(keyA);
114+
sink.getInProgressBulkLoads().remove(keyB);
115+
assertEquals(0, sink.getInProgressBulkLoads().size());
116+
}
117+
118+
/**
119+
* End-to-end: replicateEntries() with a bulkload WAL cell that has replicate=false should not
120+
* register any key in inProgressBulkLoads.
121+
*/
122+
@Test
123+
public void testNonReplicateBulkLoadNotTracked() throws Exception {
124+
WALProtos.BulkLoadDescriptor bld = buildBulkLoadDescriptor(REGION_NAME, SEQ_NUM, false);
125+
WALEdit edit = buildWALEdit(bld);
126+
List<WALEntry> entries = buildWALEntries(edit);
127+
128+
int before = sink.getInProgressBulkLoads().size();
129+
sink.replicateEntries(entries,
130+
PrivateCellUtil
131+
.createExtendedCellScanner(WALEditInternalHelper.getExtendedCells(edit).iterator()),
132+
CLUSTER_ID_A, "/dummy/namespace", "/dummy/archive");
133+
134+
assertEquals(before, sink.getInProgressBulkLoads().size(),
135+
"Non-replicate bulkload should not add key to inProgressBulkLoads");
136+
}
137+
138+
/**
139+
* Simulate a concurrent retry: manually pre-populate the key to mimic a first call still in
140+
* progress, then verify replicateEntries() skips the bulkload cell without throwing.
141+
*/
142+
@Test
143+
public void testConcurrentRetryIsSkipped() throws Exception {
144+
WALProtos.BulkLoadDescriptor bld = buildBulkLoadDescriptor(REGION_NAME, SEQ_NUM + 1, true);
145+
String key = CLUSTER_ID_A + "#" + Bytes.toString(REGION_NAME) + "#" + (SEQ_NUM + 1);
146+
147+
// Simulate first call still in progress
148+
sink.getInProgressBulkLoads().add(key);
149+
150+
WALEdit edit = buildWALEdit(bld);
151+
List<WALEntry> entries = buildWALEntries(edit);
152+
153+
// Second call (retry) should skip without exception
154+
sink.replicateEntries(entries,
155+
PrivateCellUtil
156+
.createExtendedCellScanner(WALEditInternalHelper.getExtendedCells(edit).iterator()),
157+
CLUSTER_ID_A, "/dummy/namespace", "/dummy/archive");
158+
159+
// Key still held by the "first call"
160+
assertTrue(sink.getInProgressBulkLoads().contains(key));
161+
sink.getInProgressBulkLoads().remove(key); // cleanup
162+
}
163+
164+
// ---- helpers ----
165+
166+
private WALProtos.BulkLoadDescriptor buildBulkLoadDescriptor(byte[] regionName, long seqNum,
167+
boolean replicate) {
168+
WALProtos.StoreDescriptor store =
169+
WALProtos.StoreDescriptor.newBuilder().setFamilyName(UnsafeByteOperations.unsafeWrap(FAMILY))
170+
.setStoreHomeDir(Bytes.toString(FAMILY)).addStoreFile("hfile-0").setStoreFileSizeBytes(1024)
171+
.build();
172+
return WALProtos.BulkLoadDescriptor.newBuilder()
173+
.setTableName(ProtobufUtil.toProtoTableName(TABLE))
174+
.setEncodedRegionName(UnsafeByteOperations.unsafeWrap(regionName)).addStores(store)
175+
.setBulkloadSeqNum(seqNum).setReplicate(replicate).build();
176+
}
177+
178+
private WALEdit buildWALEdit(WALProtos.BulkLoadDescriptor bld) {
179+
// RegionInfo is only used to construct the WAL cell row key; dedup logic reads
180+
// encodedRegionName from BulkLoadDescriptor directly, so any RegionInfo works here.
181+
org.apache.hadoop.hbase.client.RegionInfo ri =
182+
org.apache.hadoop.hbase.client.RegionInfoBuilder.newBuilder(TABLE).build();
183+
return WALEdit.createBulkLoadEvent(ri, bld);
184+
}
185+
186+
private List<WALEntry> buildWALEntries(WALEdit edit) {
187+
WALEntry.Builder builder = WALEntry.newBuilder();
188+
builder.setAssociatedCellCount(edit.getCells().size());
189+
WALKey.Builder keyBuilder = WALKey.newBuilder();
190+
UUID.Builder uuidBuilder = UUID.newBuilder();
191+
uuidBuilder.setLeastSigBits(HConstants.DEFAULT_CLUSTER_ID.getLeastSignificantBits());
192+
uuidBuilder.setMostSigBits(HConstants.DEFAULT_CLUSTER_ID.getMostSignificantBits());
193+
keyBuilder.setClusterId(uuidBuilder.build());
194+
keyBuilder.setTableName(UnsafeByteOperations.unsafeWrap(TABLE.getName()));
195+
keyBuilder.setWriteTime(System.currentTimeMillis());
196+
keyBuilder.setEncodedRegionName(UnsafeByteOperations.unsafeWrap(REGION_NAME));
197+
keyBuilder.setLogSequenceNumber(-1);
198+
builder.setKey(keyBuilder.build());
199+
return Collections.singletonList(builder.build());
200+
}
201+
}

0 commit comments

Comments
 (0)