Skip to content

Commit 758dc33

Browse files
authored
HBASE-29831 Fix for NPE in region replication (#7629)
Signed-off-by: Chandra Kambham <chandra@apache.org> Signed-off-by: Peng Lu <lupeng@apache.org>
1 parent 6333e25 commit 758dc33

File tree

2 files changed

+61
-3
lines changed

2 files changed

+61
-3
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,6 @@ public void append(TableName tableName, byte[] encodedRegionName, byte[] row,
427427
// invalidate the cache and check from meta
428428
RegionLocations locations = null;
429429
boolean useCache = true;
430-
int retries = 0;
431430
while (true) {
432431
// get the replicas of the primary region
433432
try {
@@ -441,12 +440,12 @@ public void append(TableName tableName, byte[] encodedRegionName, byte[] row,
441440
// keep going to the cache, we will not learn of the replicas and their locations after
442441
// they come online.
443442
if (useCache && locations.size() == 1) {
444-
if (tableDescriptors.get(tableName).getRegionReplication() > 1 && retries <= 3) {
443+
TableDescriptor td = tableDescriptors.get(tableName);
444+
if (td != null && td.getRegionReplication() > 1) {
445445
// Make an obnoxious log here. See how bad this issue is. Add a timer if happening
446446
// too much.
447447
LOG.info("Skipping location cache; only one location found for {}", tableName);
448448
useCache = false;
449-
retries++;
450449
continue;
451450
}
452451
}

hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestRegionReplicaReplicationEndpoint.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222
import static org.junit.Assert.assertNull;
2323
import static org.junit.Assert.assertTrue;
2424
import static org.junit.Assert.fail;
25+
import static org.mockito.ArgumentMatchers.any;
26+
import static org.mockito.ArgumentMatchers.anyBoolean;
27+
import static org.mockito.ArgumentMatchers.anyInt;
28+
import static org.mockito.ArgumentMatchers.eq;
2529
import static org.mockito.Mockito.mock;
2630
import static org.mockito.Mockito.when;
2731

@@ -42,7 +46,9 @@
4246
import org.apache.hadoop.hbase.HConstants;
4347
import org.apache.hadoop.hbase.HRegionLocation;
4448
import org.apache.hadoop.hbase.HTableDescriptor;
49+
import org.apache.hadoop.hbase.RegionLocations;
4550
import org.apache.hadoop.hbase.ReplicationPeerNotFoundException;
51+
import org.apache.hadoop.hbase.TableDescriptors;
4652
import org.apache.hadoop.hbase.TableName;
4753
import org.apache.hadoop.hbase.Waiter;
4854
import org.apache.hadoop.hbase.client.Admin;
@@ -557,6 +563,59 @@ public void testMetaCacheMissTriggersRefresh() throws Exception {
557563
}
558564
}
559565

566+
@Test
567+
public void testNullTableDescriptorDoesNotCauseNPE() throws Exception {
568+
TableName tableName = TableName.valueOf(name.getMethodName());
569+
int regionReplication = 2;
570+
HTableDescriptor htd = HTU.createTableDescriptor(tableName);
571+
htd.setRegionReplication(regionReplication);
572+
createOrEnableTableWithRetries(htd, true);
573+
574+
Connection connection = ConnectionFactory.createConnection(HTU.getConfiguration());
575+
Table table = connection.getTable(tableName);
576+
577+
try {
578+
HTU.loadNumericRows(table, HBaseTestingUtility.fam1, 0, 100);
579+
580+
RegionLocator rl = connection.getRegionLocator(tableName);
581+
HRegionLocation hrl = rl.getRegionLocation(HConstants.EMPTY_BYTE_ARRAY);
582+
byte[] encodedRegionName = hrl.getRegionInfo().getEncodedNameAsBytes();
583+
rl.close();
584+
585+
AtomicLong skippedEdits = new AtomicLong();
586+
RegionReplicaReplicationEndpoint.RegionReplicaOutputSink sink =
587+
mock(RegionReplicaReplicationEndpoint.RegionReplicaOutputSink.class);
588+
when(sink.getSkippedEditsCounter()).thenReturn(skippedEdits);
589+
590+
TableDescriptors mockTableDescriptors = mock(TableDescriptors.class);
591+
when(mockTableDescriptors.get(tableName)).thenReturn(null);
592+
593+
RegionLocations singleLocation = new RegionLocations(hrl);
594+
ClusterConnection mockConnection = mock(ClusterConnection.class);
595+
when(mockConnection.locateRegion(eq(tableName), any(byte[].class), anyBoolean(), anyBoolean(),
596+
anyInt())).thenReturn(singleLocation);
597+
when(mockConnection.getConfiguration()).thenReturn(HTU.getConfiguration());
598+
599+
RegionReplicaReplicationEndpoint.RegionReplicaSinkWriter sinkWriter =
600+
new RegionReplicaReplicationEndpoint.RegionReplicaSinkWriter(sink, mockConnection,
601+
Executors.newSingleThreadExecutor(), Integer.MAX_VALUE, mockTableDescriptors);
602+
603+
Cell cell = CellBuilderFactory.create(CellBuilderType.DEEP_COPY)
604+
.setRow(Bytes.toBytes("testRow")).setFamily(HBaseTestingUtility.fam1)
605+
.setValue(Bytes.toBytes("testValue")).setType(Type.Put).build();
606+
607+
Entry entry =
608+
new Entry(new WALKeyImpl(encodedRegionName, tableName, 1), new WALEdit().add(cell));
609+
610+
sinkWriter.append(tableName, encodedRegionName, Bytes.toBytes("testRow"),
611+
Lists.newArrayList(entry));
612+
613+
} finally {
614+
table.close();
615+
connection.close();
616+
}
617+
}
618+
560619
@Test
561620
public void testMetaCacheSkippedForSingleReplicaTable() throws Exception {
562621
TableName tableName = TableName.valueOf(name.getMethodName());

0 commit comments

Comments
 (0)