diff --git a/CHANGES.txt b/CHANGES.txt
index 8f0d454339f..bcd16c635c3 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -398,6 +398,8 @@ Release 0.21.0 - Unreleased
HBASE-2752 Don't retry forever when waiting on too many store files
HBASE-2737 CME in ZKW introduced in HBASE-2694 (Karthik Ranganathan via JD)
HBASE-2756 MetaScanner.metaScan doesn't take configurations
+ HBASE-2656 HMaster.getRegionTableClosest should not return null for closed
+ regions
IMPROVEMENTS
HBASE-1760 Cleanup TODOs in HTable
diff --git a/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
index d6f96115004..ee946900a51 100644
--- a/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
+++ b/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
@@ -423,6 +423,15 @@ public class HRegionInfo extends VersionedWritable implements WritableComparable
Bytes.equals(endKey, HConstants.EMPTY_BYTE_ARRAY);
return firstKeyInRange && lastKeyInRange;
}
+
+ /**
+ * Return true if the given row falls in this region.
+ */
+ public boolean containsRow(byte[] row) {
+ return Bytes.compareTo(row, startKey) >= 0 &&
+ (Bytes.compareTo(row, endKey) < 0 ||
+ Bytes.equals(endKey, HConstants.EMPTY_BYTE_ARRAY));
+ }
/** @return the tableDesc */
public HTableDescriptor getTableDesc(){
diff --git a/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java b/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
index 4c0f3929d60..bf308b3f6b9 100644
--- a/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
+++ b/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
@@ -33,8 +33,11 @@ import java.io.IOException;
* Scanner class that contains the .META. table scanning logic
* and uses a Retryable scanner. Provided visitors will be called
* for each row.
+ *
+ * Although public visibility, this is not a public-facing API and may evolve in
+ * minor releases.
*/
-class MetaScanner {
+public class MetaScanner {
/**
* Scans the meta table and calls a visitor on each RowResult and uses a empty
@@ -160,7 +163,7 @@ class MetaScanner {
/**
* Visitor class called to process each row of the .META. table
*/
- interface MetaScannerVisitor {
+ public interface MetaScannerVisitor {
/**
* Visitor method that accepts a RowResult and the meta region location.
* Implementations can return false to stop the region's loop if it becomes
diff --git a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
index 66dc6970ca3..12b9ce84f69 100644
--- a/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
+++ b/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
@@ -33,6 +33,7 @@ import java.util.NavigableMap;
import java.util.Set;
import java.util.SortedMap;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -62,6 +63,8 @@ import org.apache.hadoop.hbase.TableExistsException;
import org.apache.hadoop.hbase.client.Get;
import org.apache.hadoop.hbase.client.HBaseAdmin;
import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.MetaScanner;
+import org.apache.hadoop.hbase.client.MetaScanner.MetaScannerVisitor;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.ServerConnection;
import org.apache.hadoop.hbase.client.ServerConnectionManager;
@@ -97,6 +100,8 @@ import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.Watcher.Event.EventType;
import org.apache.zookeeper.Watcher.Event.KeeperState;
+import com.google.common.collect.Lists;
+
/**
* HMaster is the "master server" for HBase. An HBase cluster has one active
* master. If many masters are started, all compete. Whichever wins goes on to
@@ -838,99 +843,94 @@ public class HMaster extends Thread implements HMasterInterface,
new ChangeTableState(this, tableName, false).process();
}
- // TODO: Redo so this method does not duplicate code with subsequent methods.
+ /**
+ * Get a list of the regions for a given table. The pairs may have
+ * null for their second element in the case that they are not
+ * currently deployed.
+ * TODO: Redo so this method does not duplicate code with subsequent methods.
+ */
List> getTableRegions(
final byte [] tableName)
throws IOException {
- List> result =
- new ArrayList>();
- Set regions =
- this.regionManager.getMetaRegionsForTable(tableName);
- byte [] firstRowInTable = Bytes.toBytes(Bytes.toString(tableName) + ",,");
- for (MetaRegion m: regions) {
- byte [] metaRegionName = m.getRegionName();
- HRegionInterface srvr =
- this.connection.getHRegionConnection(m.getServer());
- Scan scan = new Scan(firstRowInTable);
- scan.addColumn(HConstants.CATALOG_FAMILY,
- HConstants.REGIONINFO_QUALIFIER);
- scan.addColumn(HConstants.CATALOG_FAMILY, HConstants.SERVER_QUALIFIER);
- // TODO: Use caching.
- long scannerid = srvr.openScanner(metaRegionName, scan);
- try {
- while (true) {
- Result data = srvr.next(scannerid);
+ final ArrayList> result =
+ Lists.newArrayList();
+ MetaScannerVisitor visitor =
+ new MetaScannerVisitor() {
+ @Override
+ public boolean processRow(Result data) throws IOException {
if (data == null || data.size() <= 0)
- break;
- HRegionInfo info = Writables.getHRegionInfo(
- data.getValue(HConstants.CATALOG_FAMILY,
- HConstants.REGIONINFO_QUALIFIER));
- if (Bytes.equals(info.getTableDesc().getName(), tableName)) {
- final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
- HConstants.SERVER_QUALIFIER);
- if (value != null && value.length > 0) {
- HServerAddress server = new HServerAddress(Bytes.toString(value));
- result.add(new Pair(info, server));
- }
- } else {
- break;
+ return true;
+ Pair pair =
+ metaRowToRegionPair(data);
+ if (pair == null) return false;
+ if (!Bytes.equals(pair.getFirst().getTableDesc().getName(),
+ tableName)) {
+ return false;
}
+ result.add(pair);
+ return true;
}
- } finally {
- srvr.close(scannerid);
- }
- }
+ };
+
+ MetaScanner.metaScan(conf, visitor, tableName);
return result;
}
-
- Pair getTableRegionClosest(
- final byte [] tableName, final byte [] rowKey)
- throws IOException {
- Set regions =
- this.regionManager.getMetaRegionsForTable(tableName);
- for (MetaRegion m: regions) {
- byte [] firstRowInTable = Bytes.toBytes(Bytes.toString(tableName) + ",,");
- byte [] metaRegionName = m.getRegionName();
- HRegionInterface srvr = this.connection.getHRegionConnection(m.getServer());
- Scan scan = new Scan(firstRowInTable);
- scan.addColumn(HConstants.CATALOG_FAMILY,
- HConstants.REGIONINFO_QUALIFIER);
- scan.addColumn(HConstants.CATALOG_FAMILY, HConstants.SERVER_QUALIFIER);
- long scannerid = srvr.openScanner(metaRegionName, scan);
- try {
- while (true) {
- Result data = srvr.next(scannerid);
- if (data == null || data.size() <= 0)
- break;
- HRegionInfo info = Writables.getHRegionInfo(
- data.getValue(HConstants.CATALOG_FAMILY,
- HConstants.REGIONINFO_QUALIFIER));
- if (Bytes.compareTo(info.getTableDesc().getName(), tableName) == 0) {
- if ((Bytes.compareTo(info.getStartKey(), rowKey) >= 0) &&
- (Bytes.compareTo(info.getEndKey(), rowKey) < 0)) {
- final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
- HConstants.SERVER_QUALIFIER);
- if (value != null && value.length > 0) {
- HServerAddress server =
- new HServerAddress(Bytes.toString(value));
- return new Pair(info, server);
- }
- }
- } else {
- break;
- }
- }
- } finally {
- srvr.close(scannerid);
- }
- }
- return null;
+
+ private Pair metaRowToRegionPair(
+ Result data) throws IOException {
+ HRegionInfo info = Writables.getHRegionInfo(
+ data.getValue(HConstants.CATALOG_FAMILY,
+ HConstants.REGIONINFO_QUALIFIER));
+ final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
+ HConstants.SERVER_QUALIFIER);
+ if (value != null && value.length > 0) {
+ HServerAddress server = new HServerAddress(Bytes.toString(value));
+ return new Pair(info, server);
+ } else {
+ //undeployed
+ return new Pair(info, null);
+ }
}
+ /**
+ * Return the region and current deployment for the region containing
+ * the given row. If the region cannot be found, returns null. If it
+ * is found, but not currently deployed, the second element of the pair
+ * may be null.
+ */
+ Pair getTableRegionForRow(
+ final byte [] tableName, final byte [] rowKey)
+ throws IOException {
+ final AtomicReference> result =
+ new AtomicReference>(null);
+
+ MetaScannerVisitor visitor =
+ new MetaScannerVisitor() {
+ @Override
+ public boolean processRow(Result data) throws IOException {
+ if (data == null || data.size() <= 0)
+ return true;
+ Pair pair =
+ metaRowToRegionPair(data);
+ if (pair == null) return false;
+ if (!Bytes.equals(pair.getFirst().getTableDesc().getName(),
+ tableName)) {
+ return false;
+ }
+ result.set(pair);
+ return true;
+ }
+ };
+
+ MetaScanner.metaScan(conf, visitor, tableName, rowKey, 1);
+ return result.get();
+ }
+
Pair getTableRegionFromName(
final byte [] regionName)
throws IOException {
byte [] tableName = HRegionInfo.parseRegionName(regionName)[0];
+
Set regions = regionManager.getMetaRegionsForTable(tableName);
for (MetaRegion m: regions) {
byte [] metaRegionName = m.getRegionName();
@@ -941,16 +941,7 @@ public class HMaster extends Thread implements HMasterInterface,
get.addColumn(HConstants.CATALOG_FAMILY, HConstants.SERVER_QUALIFIER);
Result data = srvr.get(metaRegionName, get);
if(data == null || data.size() <= 0) continue;
- HRegionInfo info = Writables.getHRegionInfo(
- data.getValue(HConstants.CATALOG_FAMILY,
- HConstants.REGIONINFO_QUALIFIER));
- final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
- HConstants.SERVER_QUALIFIER);
- if(value != null && value.length > 0) {
- HServerAddress server =
- new HServerAddress(Bytes.toString(value));
- return new Pair(info, server);
- }
+ return metaRowToRegionPair(data);
}
return null;
}
@@ -1008,16 +999,18 @@ public class HMaster extends Thread implements HMasterInterface,
pair = getTableRegionFromName(regionName);
} else {
byte [] rowKey = ((ImmutableBytesWritable)args[0]).get();
- pair = getTableRegionClosest(tableName, rowKey);
+ pair = getTableRegionForRow(tableName, rowKey);
}
- if (pair != null) {
+ if (pair != null && pair.getSecond() != null) {
this.regionManager.startAction(pair.getFirst().getRegionName(),
pair.getFirst(), pair.getSecond(), op);
}
} else {
- for (Pair pair: getTableRegions(tableName))
+ for (Pair pair: getTableRegions(tableName)) {
+ if (pair.getSecond() == null) continue; // undeployed
this.regionManager.startAction(pair.getFirst().getRegionName(),
pair.getFirst(), pair.getSecond(), op);
+ }
}
break;
diff --git a/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java b/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
index a5f4d6969a3..01ae0d241eb 100644
--- a/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
+++ b/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.MiniHBaseCluster;
import org.apache.hadoop.hbase.HMsg;
import org.apache.hadoop.hbase.HServerInfo;
@@ -36,12 +37,16 @@ import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.Pair;
import java.io.IOException;
+import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
+
+import com.google.common.base.Joiner;
+
import static org.junit.Assert.*;
public class TestMaster {
@@ -70,30 +75,38 @@ public class TestMaster {
TEST_UTIL.createTable(TABLENAME, FAMILYNAME);
TEST_UTIL.loadTable(new HTable(TABLENAME), FAMILYNAME);
+ List> tableRegions =
+ m.getTableRegions(TABLENAME);
+ LOG.info("Regions after load: " + Joiner.on(',').join(tableRegions));
+ assertEquals(1, tableRegions.size());
+ assertArrayEquals(HConstants.EMPTY_START_ROW,
+ tableRegions.get(0).getFirst().getStartKey());
+ assertArrayEquals(HConstants.EMPTY_END_ROW,
+ tableRegions.get(0).getFirst().getEndKey());
+
+ // Now trigger a split and stop when the split is in progress
+
CountDownLatch aboutToOpen = new CountDownLatch(1);
CountDownLatch proceed = new CountDownLatch(1);
RegionOpenListener list = new RegionOpenListener(aboutToOpen, proceed);
HBaseEventHandler.registerListener(list);
+ LOG.info("Splitting table");
admin.split(TABLENAME);
+ LOG.info("Waiting for split result to be about to open");
aboutToOpen.await(60, TimeUnit.SECONDS);
-
try {
- m.getTableRegions(TABLENAME);
+ LOG.info("Making sure we can call getTableRegions while opening");
+ tableRegions = m.getTableRegions(TABLENAME);
+ LOG.info("Regions: " + Joiner.on(',').join(tableRegions));
+ // We have three regions because one is split-in-progress
+ assertEquals(3, tableRegions.size());
+ LOG.info("Making sure we can call getTableRegionClosest while opening");
Pair pair =
- m.getTableRegionClosest(TABLENAME, Bytes.toBytes("cde"));
- /**
- * TODO: The assertNull below used to work before moving all RS->M
- * communication to ZK, find out why this test's behavior has changed.
- * Tracked in HBASE-2656.
- assertNull(pair);
- assertNotNull(pair);
-
- We used to assert NotNull for the pair but it seems that ain't
- always true either. For now disabling this assertion. Filing
- an issue for it to be checked -- St.Ack.
- */
- m.getTableRegionFromName(pair.getFirst().getRegionName());
+ m.getTableRegionForRow(TABLENAME, Bytes.toBytes("cde"));
+ LOG.info("Result is: " + pair);
+ Pair tableRegionFromName = m.getTableRegionFromName(pair.getFirst().getRegionName());
+ assertEquals(tableRegionFromName.getFirst(), pair.getFirst());
} finally {
proceed.countDown();
}