saintstack commented on a change in pull request #1774:
URL: https://github.com/apache/hbase/pull/1774#discussion_r437021705
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##########
@@ -2319,17 +2317,13 @@ private boolean skipReportingTransition(final
RegionStateTransitionContext conte
if (code == TransitionCode.OPENED) {
Preconditions.checkArgument(hris != null && hris.length == 1);
if (hris[0].isMetaRegion()) {
- try {
- MetaTableLocator.setMetaLocation(getZooKeeper(), serverName,
- hris[0].getReplicaId(), RegionState.State.OPEN);
- } catch (KeeperException e) {
- LOG.info("Failed to update meta location", e);
- return false;
- }
+ LOG.warn(
+ "meta table location is stored in master local store, so we can not
skip reporting");
Review comment:
master local region
##########
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:
Too later to changes this?
##########
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:
Does MasterRegistry put meta location in zk? If not, we'd mirror to zk
here anyways even if we are using MasterRegistry?
##########
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:
MasterRegion deserves own accessor. MasterRegionAccessor
##########
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:
The api does not allow 'paging' through the hbase:master table Regions.
Even if we prefetch the API is awkward for getting next Region.
If a paging API, we'd only need one method?
I think current situation where we only get one Region because there is only
one makes this API seem ok.
##########
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:
We'd not get this info from the Connection via Registry? Or is it that
this is pure internal to Master? Reads the Master local Region content? If so,
why the param? What internal needs this?
##########
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:
TABLENAME is 'master:store'. Should it be 'master:region'? Configs are
hbase.master.store.region.* which is odd given we usually talk of region having
a store, not other way around.
##########
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:
We don't need to make sure meta is available first anymore? Or is this
because we might be doing create meta table with this procedure going forward?
##########
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:
Too late to changes this?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
##########
@@ -1105,23 +1097,7 @@ public void run(PRESP resp) {
* List all region locations for the specific table.
*/
private CompletableFuture<List<HRegionLocation>>
getTableHRegionLocations(TableName tableName) {
- if (TableName.META_TABLE_NAME.equals(tableName)) {
- CompletableFuture<List<HRegionLocation>> future = new
CompletableFuture<>();
- addListener(connection.registry.getMetaRegionLocations(), (metaRegions,
err) -> {
- if (err != null) {
- future.completeExceptionally(err);
- } else if (metaRegions == null || metaRegions.isEmpty() ||
- metaRegions.getDefaultRegionLocation() == null) {
- future.completeExceptionally(new IOException("meta region does not
found"));
- } else {
-
future.complete(Collections.singletonList(metaRegions.getDefaultRegionLocation()));
- }
- });
- return future;
- } else {
- // For non-meta table, we fetch all locations by scanning hbase:meta
table
- return AsyncMetaTableAccessor.getTableHRegionLocations(metaTable,
tableName);
- }
+ return connection.getRegionLocator(tableName).getAllRegionLocations();
Review comment:
Looks good.
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
##########
@@ -3537,4 +3538,30 @@ public static RSGroupInfo
toGroupInfo(RSGroupProtos.RSGroupInfo proto) {
return
RSGroupProtos.RSGroupInfo.newBuilder().setName(pojo.getName()).addAllServers(hostports)
.addAllTables(tables).build();
}
+
+ public static MasterProtos.RegionLocateType
toProtoRegionLocateType(RegionLocateType pojo) {
+ switch (pojo) {
+ case BEFORE:
+ return MasterProtos.RegionLocateType.REGION_LOCATE_TYPE_BEFORE;
+ case CURRENT:
+ return MasterProtos.RegionLocateType.REGION_LOCATE_TYPE_CURRENT;
+ case AFTER:
+ return MasterProtos.RegionLocateType.REGION_LOCATE_TYPE_AFTER;
+ default:
+ throw new IllegalArgumentException("Unknown RegionLocateType: " +
pojo);
+ }
+ }
+
+ public static RegionLocateType
toRegionLocateType(MasterProtos.RegionLocateType proto) {
+ switch (proto) {
+ case REGION_LOCATE_TYPE_BEFORE:
+ return RegionLocateType.BEFORE;
+ case REGION_LOCATE_TYPE_CURRENT:
+ return RegionLocateType.CURRENT;
+ case REGION_LOCATE_TYPE_AFTER:
+ return RegionLocateType.AFTER;
+ default:
+ throw new IllegalArgumentException("Unknown proto RegionLocateType: "
+ proto);
+ }
+ }
Review comment:
Could you transport this as a Scan? Convert the 'simple API' into a Scan
behind the scenes so we don't have to do stuff like this?
(I hear you on not wanting to support full Scan and have some sympathy but I
am kicking against a simple API that seems arbitrary and unsatisfactory --
hence the questions. Pardon if annoying... also trying to figure what is going
on in here... Takes me a while. I'm slow. Thanks).
----------------------------------------------------------------
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:
us...@infra.apache.org
