Apache9 commented on a change in pull request #1774:
URL: https://github.com/apache/hbase/pull/1774#discussion_r437077771
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -3921,4 +3996,85 @@ public MetaRegionLocationCache
getMetaRegionLocationCache() {
public RSGroupInfoManager getRSGroupInfoManager() {
return rsGroupInfoManager;
}
+
+ public RegionLocations locateMeta(byte[] row, RegionLocateType locateType)
throws IOException {
+ if (locateType == RegionLocateType.AFTER) {
+ // as we know the exact row after us, so we can just create the new row,
and use the same
+ // algorithm to locate it.
+ row = Arrays.copyOf(row, row.length + 1);
+ locateType = RegionLocateType.CURRENT;
+ }
+ Scan scan =
+ MetaTableAccessor.createLocateRegionScan(TableName.META_TABLE_NAME, row,
locateType, 1);
+ try (RegionScanner scanner = masterRegion.getScanner(scan)) {
+ boolean moreRows;
+ List<Cell> cells = new ArrayList<>();
+ do {
+ moreRows = scanner.next(cells);
+ if (cells.isEmpty()) {
+ continue;
+ }
+ Result result = Result.create(cells);
+ cells.clear();
+ RegionLocations locs = MetaTableAccessor.getRegionLocations(result);
+ if (locs == null || locs.getDefaultRegionLocation() == null) {
+ LOG.warn("No location found when locating meta region with row='{}',
locateType={}",
+ Bytes.toStringBinary(row), locateType);
+ return null;
+ }
+ HRegionLocation loc = locs.getDefaultRegionLocation();
+ RegionInfo info = loc.getRegion();
+ if (info == null) {
+ LOG.warn("HRegionInfo is null when locating meta region with
row='{}', locateType={}",
+ Bytes.toStringBinary(row), locateType);
+ return null;
+ }
+ if (info.isSplitParent()) {
+ continue;
+ }
+ return locs;
+ } while (moreRows);
+ LOG.warn("No location available when locating meta region with row='{}',
locateType={}",
+ Bytes.toStringBinary(row), locateType);
+ return null;
+ }
+ }
+
+ public List<RegionLocations> getAllMetaRegionLocations(boolean
excludeOfflinedSplitParents)
+ throws IOException {
+ Scan scan = new Scan().addFamily(HConstants.CATALOG_FAMILY);
+ List<RegionLocations> list = new ArrayList<>();
+ try (RegionScanner scanner = masterRegion.getScanner(scan)) {
+ boolean moreRows;
+ List<Cell> cells = new ArrayList<>();
+ do {
+ moreRows = scanner.next(cells);
+ if (cells.isEmpty()) {
+ continue;
+ }
+ Result result = Result.create(cells);
+ cells.clear();
+ RegionLocations locs = MetaTableAccessor.getRegionLocations(result);
+ if (locs == null) {
+ LOG.warn("No locations in {}", result);
+ continue;
+ }
+ HRegionLocation loc = locs.getRegionLocation();
+ if (loc == null) {
+ LOG.warn("No non null location in {}", result);
+ continue;
+ }
+ RegionInfo info = loc.getRegion();
+ if (info == null) {
+ LOG.warn("No serialized RegionInfo in {}", result);
+ continue;
+ }
+ if (excludeOfflinedSplitParents && info.isSplitParent()) {
+ continue;
+ }
+ list.add(locs);
+ } while (moreRows);
+ }
+ return list;
+ }
Review comment:
Can do this in a follow on issue? And the difference here is that,
MasterRegion is not only used as root table, it is also usedas procedure store,
which does not have the same access pattern.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegionFactory.java
##########
@@ -80,6 +83,10 @@
public static final byte[] PROC_FAMILY = Bytes.toBytes("proc");
Review comment:
I think the 'store' here does not mean HRegion, it is something like the
'ProcedureStore'.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
##########
@@ -224,23 +229,52 @@ public void start() throws IOException, KeeperException {
// Start the Assignment Thread
startAssignmentThread();
- // load meta region state
- ZKWatcher zkw = master.getZooKeeper();
- // it could be null in some tests
- if (zkw != null) {
- RegionState regionState = MetaTableLocator.getMetaRegionState(zkw);
- RegionStateNode regionNode =
-
regionStates.getOrCreateRegionStateNode(RegionInfoBuilder.FIRST_META_REGIONINFO);
- regionNode.lock();
- try {
- regionNode.setRegionLocation(regionState.getServerName());
- regionNode.setState(regionState.getState());
- if (regionNode.getProcedure() != null) {
- regionNode.getProcedure().stateLoaded(this, regionNode);
+ // load meta region states.
+ // notice that, here we will load all replicas, and in MasterMetaBootstrap
we may assign new
+ // replicas, or remove excess replicas.
+ try (RegionScanner scanner =
+ masterRegion.getScanner(new
Scan().addFamily(HConstants.CATALOG_FAMILY))) {
+ List<Cell> cells = new ArrayList<>();
+ boolean moreRows;
+ do {
+ moreRows = scanner.next(cells);
+ if (cells.isEmpty()) {
+ continue;
}
- setMetaAssigned(regionState.getRegion(), regionState.getState() ==
State.OPEN);
- } finally {
- regionNode.unlock();
+ Result result = Result.create(cells);
+ cells.clear();
+ RegionStateStore
+ .visitMetaEntry((r, regionInfo, state, regionLocation, lastHost,
openSeqNum) -> {
+ RegionStateNode regionNode =
regionStates.getOrCreateRegionStateNode(regionInfo);
+ regionNode.lock();
+ try {
+ regionNode.setState(state);
+ regionNode.setLastHost(lastHost);
+ regionNode.setRegionLocation(regionLocation);
+ regionNode.setOpenSeqNum(openSeqNum);
+ if (regionNode.getProcedure() != null) {
+ regionNode.getProcedure().stateLoaded(this, regionNode);
+ }
+ if (RegionReplicaUtil.isDefaultReplica(regionInfo)) {
+ setMetaAssigned(regionInfo, state == State.OPEN);
+ }
+ } finally {
+ regionNode.unlock();
+ }
+ if (regionInfo.isFirst()) {
+ // for compatibility, mirror the meta region state to zookeeper
+ try {
+ regionStateStore.mirrorMetaLocation(regionInfo,
regionLocation, state);
Review comment:
As said earlier, ConnectionRegistry is only a read only interface used
at client side, where MasterRegsitry is an implementation of
ConnectionRegistry, so it will not write anything to zk. The mirror of the
location here is to keep compatible with old clients, where they expect meta
location is still on zk.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
##########
@@ -552,4 +553,12 @@ default SplitWALManager getSplitWALManager(){
* @return The state of the load balancer, or false if the load balancer
isn't defined.
*/
boolean isBalancerOn();
+
+ /**
+ * Get locations for all meta regions.
+ * @param excludeOfflinedSplitParents don't return split parents
+ * @return The locations of all the meta regions
+ */
+ List<RegionLocations> getAllMetaRegionLocations(boolean
excludeOfflinedSplitParents)
Review comment:
The ConnectionRegistry need to call this method to get the location. I
still need to say, ConnectionRegistry is an interface which is only used at
client side, it is not something at the bottom to support the cluster. Now the
location of meta is stored in a master local region, you have to provide an rpc
method for ConnectionRegistry to get the location...
On the parameter, maybe we do not need it in the rpc interface, for now, as
in the RegionLocator interface there is no way to pass this parameter. But when
doing snapshot, we do need to get the split parent. And maybe for HBCK we also
need to get these things? Not sure, if you do not like it we can remove it. Can
add it back later when necessary.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
##########
@@ -365,8 +365,6 @@ protected static void moveTempDirectoryToHBaseRoot(
final List<RegionInfo> regions) throws IOException {
assert (regions != null && regions.size() > 0) : "expected at least 1
region, got " + regions;
- ProcedureSyncWait.waitMetaRegions(env);
Review comment:
If we have multiple meta regions, I do not think it is a good idea to
wait for them all being online? And the implementation will be much cmore
omplicated, as in the past we just need a simple event and setMetaAssigned and
setMetaUnassigned. But if we have multiple meta regions, we need to know the
exact count of meta regions and do counting here?And we also need to deal with
split/merge which could change the count.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegionFactory.java
##########
@@ -80,6 +83,10 @@
public static final byte[] PROC_FAMILY = Bytes.toBytes("proc");
Review comment:
Maybe later we have other implementations then we will change the
configs to hbase.master.store.<alternate implementation>,*
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/region/MasterRegionFactory.java
##########
@@ -80,6 +83,10 @@
public static final byte[] PROC_FAMILY = Bytes.toBytes("proc");
Review comment:
Maybe later we have other implementations then we will change the
configs to hbase.master.store.<alternate implementation>,*
##########
File path: hbase-protocol-shaded/src/main/protobuf/server/master/Master.proto
##########
@@ -1284,4 +1307,16 @@ service ClientMetaService {
* Get current meta replicas' region locations.
*/
rpc GetMetaRegionLocations(GetMetaRegionLocationsRequest)
returns(GetMetaRegionLocationsResponse);
+
+ /**
+ * Get meta region locations for a given row
+ */
+ rpc LocateMetaRegion(LocateMetaRegionRequest)
+ returns(LocateMetaRegionResponse);
+
+ /**
+ * Get all meta regions locations
+ */
+ rpc GetAllMetaRegionLocations(GetAllMetaRegionLocationsRequest)
+ returns(GetAllMetaRegionLocationsResponse);
Review comment:
There is a locateMeta method, which is to be used at the normal
read/write path, to support locating a meta region with specific key. And what
I said above, about the prefetch, is to add a prefetch parameter for the
locateMeta method, not for the GetAllMetaRegionLocations method.
The GetAllMetaRegionLocations method are mainly designed to have two usages,
one is for other cache servers to pull all the content so it can serve the
locateMeta requests from client, another is for some maintainance methods, such
as RegionLocator.getAllRegionLocations. We should not call this method for in
normal read write path.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
##########
@@ -2185,4 +2206,27 @@ private static Put addSequenceNum(Put p, long
openSeqNum, int replicaId) throws
.setValue(Bytes.toBytes(openSeqNum))
.build());
}
+
+ private static byte[] buildLocateRegionStartRow(TableName tableName, byte[]
row,
+ RegionLocateType locateType) {
+ if (locateType.equals(RegionLocateType.BEFORE)) {
+ if (Bytes.equals(row, HConstants.EMPTY_END_ROW)) {
+ byte[] binaryTableName = tableName.getName();
+ return Arrays.copyOf(binaryTableName, binaryTableName.length + 1);
+ } else {
+ return createRegionName(tableName, row, ZEROES, false);
+ }
+ } else {
+ return createRegionName(tableName, row, NINES, false);
+ }
+ }
+
+ public static Scan createLocateRegionScan(TableName tableName, byte[] row,
+ RegionLocateType locateType, int prefetchLimit) {
Review comment:
We could be better orginzation of these methods in follow on patches,
i.e, moving this method to other places to not mess up the MetaTableAccessor. I
do not have strong feelings that this method must be put here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
