You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by nd...@apache.org on 2024/02/29 17:11:31 UTC
(hbase) branch branch-2.6 updated: HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition (#5699)
This is an automated email from the ASF dual-hosted git repository.
ndimiduk pushed a commit to branch branch-2.6
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.6 by this push:
new 91672904a7b HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition (#5699)
91672904a7b is described below
commit 91672904a7ba86c80cfe3c841ac25e79e5342ef4
Author: Ahmad Alhour <aa...@users.noreply.github.com>
AuthorDate: Thu Feb 29 14:55:25 2024 +0100
HBASE-28354 RegionSizeCalculator throws NPE when regions are in transition (#5699)
When a region is in transition, it may briefly have a null ServerName in meta. The
RegionSizeCalculator calls RegionLocator.getAllRegionLocations() and does not handle the
possibility that a RegionLocation.getServerName() could be null. The ServerName is eventually
passed into an Admin call, which results in an NPE.
This has come up in other contexts. For example, taking a look at getAllRegionLocations() impl, we
have checks to ensure that we don't call null server names. We need to similarly handle the
possibility of nulls in RegionSizeCalculator.
Signed-off-by: Nick Dimiduk <nd...@apache.org>
Signed-off-by: Hui Ruan <hu...@apache.org>
---
.../hbase/mapreduce/RegionSizeCalculator.java | 15 ++++----
.../hbase/mapreduce/TestRegionSizeCalculator.java | 40 +++++++++++++++-------
2 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java b/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java
index 4d027196a8f..cc36ef5deb4 100644
--- a/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java
+++ b/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java
@@ -21,8 +21,10 @@ import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.TreeMap;
+import java.util.stream.Collectors;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.RegionMetrics;
@@ -35,8 +37,6 @@ import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
-
/**
* Computes size of each region for given table and given column families. The value is used by
* MapReduce for better scheduling.
@@ -96,12 +96,11 @@ public class RegionSizeCalculator {
}
private Set<ServerName> getRegionServersOfTable(RegionLocator regionLocator) throws IOException {
-
- Set<ServerName> tableServers = Sets.newHashSet();
- for (HRegionLocation regionLocation : regionLocator.getAllRegionLocations()) {
- tableServers.add(regionLocation.getServerName());
- }
- return tableServers;
+ // The region locations could contain `null` ServerName instances if the region is currently
+ // in transition, we filter those out for now, which impacts the size calculation for these
+ // regions temporarily until the ServerName gets filled in later
+ return regionLocator.getAllRegionLocations().stream().map(HRegionLocation::getServerName)
+ .filter(Objects::nonNull).collect(Collectors.toSet());
}
boolean enabled(Configuration configuration) {
diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestRegionSizeCalculator.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestRegionSizeCalculator.java
index dab81245e9d..90ea71070fe 100644
--- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestRegionSizeCalculator.java
+++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestRegionSizeCalculator.java
@@ -23,6 +23,8 @@ import static org.mockito.Mockito.when;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -36,6 +38,7 @@ import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.RegionLocator;
import org.apache.hadoop.hbase.testclassification.MiscTests;
import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.Bytes;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -63,11 +66,12 @@ public class TestRegionSizeCalculator {
RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
- assertEquals(123 * megabyte, calculator.getRegionSize("region1".getBytes()));
- assertEquals(54321 * megabyte, calculator.getRegionSize("region2".getBytes()));
- assertEquals(1232 * megabyte, calculator.getRegionSize("region3".getBytes()));
+ assertEquals(123 * megabyte, calculator.getRegionSize(Bytes.toBytes("region1")));
+ assertEquals(54321 * megabyte, calculator.getRegionSize(Bytes.toBytes("region2")));
+ assertEquals(1232 * megabyte, calculator.getRegionSize(Bytes.toBytes("region3")));
+
// if regionCalculator does not know about a region, it should return 0
- assertEquals(0 * megabyte, calculator.getRegionSize("otherTableRegion".getBytes()));
+ assertEquals(0, calculator.getRegionSize(Bytes.toBytes("otherTableRegion")));
assertEquals(3, calculator.getRegionSizeMap().size());
}
@@ -104,24 +108,37 @@ public class TestRegionSizeCalculator {
// then disabled calculator.
configuration.setBoolean(RegionSizeCalculator.ENABLE_REGIONSIZECALCULATOR, false);
RegionSizeCalculator disabledCalculator = new RegionSizeCalculator(table, admin);
- assertEquals(0 * megabyte, disabledCalculator.getRegionSize(regionName.getBytes()));
-
+ assertEquals(0, disabledCalculator.getRegionSize(Bytes.toBytes(regionName)));
assertEquals(0, disabledCalculator.getRegionSizeMap().size());
}
+ @Test
+ public void testRegionWithNullServerName() throws Exception {
+ RegionLocator regionLocator =
+ mockRegionLocator(null, Collections.singletonList("someBigRegion"));
+ Admin admin = mockAdmin(mockRegion("someBigRegion", Integer.MAX_VALUE));
+ RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
+ assertEquals(0, calculator.getRegionSize(Bytes.toBytes("someBigRegion")));
+ }
+
/**
* Makes some table with given region names.
*/
private RegionLocator mockRegionLocator(String... regionNames) throws IOException {
+ return mockRegionLocator(sn, Arrays.asList(regionNames));
+ }
+
+ private RegionLocator mockRegionLocator(ServerName serverName, List<String> regionNames)
+ throws IOException {
RegionLocator mockedTable = Mockito.mock(RegionLocator.class);
when(mockedTable.getName()).thenReturn(TableName.valueOf("sizeTestTable"));
- List<HRegionLocation> regionLocations = new ArrayList<>(regionNames.length);
+ List<HRegionLocation> regionLocations = new ArrayList<>(regionNames.size());
when(mockedTable.getAllRegionLocations()).thenReturn(regionLocations);
for (String regionName : regionNames) {
HRegionInfo info = Mockito.mock(HRegionInfo.class);
- when(info.getRegionName()).thenReturn(regionName.getBytes());
- regionLocations.add(new HRegionLocation(info, sn));
+ when(info.getRegionName()).thenReturn(Bytes.toBytes(regionName));
+ regionLocations.add(new HRegionLocation(info, serverName));
}
return mockedTable;
@@ -132,10 +149,7 @@ public class TestRegionSizeCalculator {
*/
private Admin mockAdmin(RegionMetrics... regionLoadArray) throws Exception {
Admin mockAdmin = Mockito.mock(Admin.class);
- List<RegionMetrics> regionLoads = new ArrayList<>();
- for (RegionMetrics regionLoad : regionLoadArray) {
- regionLoads.add(regionLoad);
- }
+ List<RegionMetrics> regionLoads = new ArrayList<>(Arrays.asList(regionLoadArray));
when(mockAdmin.getConfiguration()).thenReturn(configuration);
when(mockAdmin.getRegionMetrics(sn, TableName.valueOf("sizeTestTable")))
.thenReturn(regionLoads);