Apache9 commented on code in PR #4664: URL: https://github.com/apache/hbase/pull/4664#discussion_r938383599
########## hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java: ########## @@ -2479,32 +2481,42 @@ public SetRegionStateInMetaResponse setRegionStateInMeta(RpcController controlle for (RegionSpecifierAndState s : request.getStatesList()) { RegionSpecifier spec = s.getRegionSpecifier(); String encodedName; + RegionInfo info; + int replicaId; if (spec.getType() == RegionSpecifierType.ENCODED_REGION_NAME) { - encodedName = spec.getValue().toStringUtf8(); + info = this.server.getAssignmentManager() + .getRegionInfoFromEncodedRegionName(spec.getValue().toStringUtf8()); } else { // TODO: actually, a full region name can save a lot on meta scan, improve later. - encodedName = RegionInfo.encodeRegionName(spec.getValue().toByteArray()); + info = CatalogFamilyFormat.parseRegionInfoFromRegionName(spec.getValue().toByteArray()); } - RegionInfo info = this.server.getAssignmentManager().loadRegionFromMeta(encodedName); - LOG.trace("region info loaded from meta table: {}", info); + replicaId = info.getReplicaId(); + LOG.trace("region info", info); RegionState prevState = this.server.getAssignmentManager().getRegionStates().getRegionState(info); RegionState.State newState = RegionState.State.convert(s.getState()); LOG.info("{} set region={} state from {} to {}", server.getClientIdAuditPrefix(), info, prevState.getState(), newState); - Put metaPut = - MetaTableAccessor.makePutFromRegionInfo(info, EnvironmentEdgeManager.currentTime()); - metaPut.addColumn(HConstants.CATALOG_FAMILY, HConstants.STATE_QUALIFIER, - Bytes.toBytes(newState.name())); - List<Put> putList = new ArrayList<>(); - putList.add(metaPut); - MetaTableAccessor.putsToMetaTable(this.server.getConnection(), putList); - // Loads from meta again to refresh AM cache with the new region state - this.server.getAssignmentManager().loadRegionFromMeta(encodedName); - builder.addStates(RegionSpecifierAndState.newBuilder().setRegionSpecifier(spec) - .setState(prevState.getState().convert())); + // If state does not change, no need to set. + if (prevState.getState() != newState) { + if (replicaId > RegionInfo.DEFAULT_REPLICA_ID) { + // If it is a non-primary replica region, use primary region as the key. + info = RegionInfoBuilder.newBuilder(info).setReplicaId(RegionInfo.DEFAULT_REPLICA_ID) Review Comment: Is the region info passed from region name have all the fields filled? I think we should always get the info from AM and only change its state field? ########## hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java: ########## @@ -2479,32 +2481,42 @@ public SetRegionStateInMetaResponse setRegionStateInMeta(RpcController controlle for (RegionSpecifierAndState s : request.getStatesList()) { RegionSpecifier spec = s.getRegionSpecifier(); String encodedName; + RegionInfo info; + int replicaId; if (spec.getType() == RegionSpecifierType.ENCODED_REGION_NAME) { - encodedName = spec.getValue().toStringUtf8(); + info = this.server.getAssignmentManager() + .getRegionInfoFromEncodedRegionName(spec.getValue().toStringUtf8()); } else { // TODO: actually, a full region name can save a lot on meta scan, improve later. - encodedName = RegionInfo.encodeRegionName(spec.getValue().toByteArray()); + info = CatalogFamilyFormat.parseRegionInfoFromRegionName(spec.getValue().toByteArray()); Review Comment: The TODO can be removed now? We do use the full name now... -- 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. To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org