Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-23313 [hbck2] setRegionState should update Master in-memory sta… #864

Merged
merged 5 commits into from
Nov 27, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,17 @@

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.stream.Collectors;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.ipc.RpcControllerFactory;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException;
import org.apache.hadoop.conf.Configuration;

import org.apache.hadoop.hbase.ipc.RpcControllerFactory;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
import org.apache.hadoop.hbase.shaded.protobuf.RequestConverter;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.AssignsResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.BypassProcedureRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.BypassProcedureResponse;
Expand All @@ -45,6 +41,13 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ScheduleServerCrashProcedureResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.UnassignsResponse;

import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException;

import org.apache.yetus.audience.InterfaceAudience;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Use {@link Connection#getHbck()} to obtain an instance of {@link Hbck} instead of
* constructing an HBaseHbck directly.
Expand Down Expand Up @@ -98,15 +101,29 @@ public boolean isAborted() {
public TableState setTableStateInMeta(TableState state) throws IOException {
try {
GetTableStateResponse response = hbck.setTableStateInMeta(
rpcControllerFactory.newController(),
RequestConverter.buildSetTableStateInMetaRequest(state));
rpcControllerFactory.newController(),
RequestConverter.buildSetTableStateInMetaRequest(state));
return TableState.convert(state.getTableName(), response.getTableState());
} catch (ServiceException se) {
LOG.debug("table={}, state={}", state.getTableName(), state.getState(), se);
throw new IOException(se);
}
}

@Override
public RegionState setRegionStateInMeta(RegionState state) throws IOException {
try {
LOG.trace("RegionInfo from state: {}", state.getRegion().getEncodedName());
MasterProtos.GetRegionStateResponse response = hbck.setRegionStateInMeta(
rpcControllerFactory.newController(),
RequestConverter.buildSetRegionStateInMetaRequest(state));
return RegionState.convert(response.getRegionState());
} catch (ServiceException se) {
LOG.debug("region={}, state={}", state.getRegion().getRegionName(), state.getState(), se);
throw new IOException(se);
}
}

@Override
public List<Long> assigns(List<String> encodedRegionNames, boolean override)
throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
Expand Down Expand Up @@ -54,6 +55,14 @@ public interface Hbck extends Abortable, Closeable {
*/
TableState setTableStateInMeta(TableState state) throws IOException;

/**
* Update region state in Meta only. No procedures are submitted to manipulate the given region
* or any other region from same table.
* @param state region state
* @return previous state of the region in Meta
*/
RegionState setRegionStateInMeta(RegionState state) throws IOException;

/**
* Like {@link Admin#assign(byte[])} but 'raw' in that it can do more than one Region at a time
* -- good if many Regions to online -- and it will schedule the assigns even in the case where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ public RegionInfoBuilder setOffline(boolean offLine) {
return this;
}

public RegionInfoBuilder setEncodedName(String encodedName) {
this.encodedName = encodedName;
return this;
}

public RegionInfo build() {
return new MutableRegionInfo(tableName, startKey, endKey, split,
regionId, replicaId, offLine, regionName, encodedName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,14 @@ public static HBaseProtos.TableName toProtoTableName(TableName tableName) {
.setQualifier(UnsafeByteOperations.unsafeWrap(tableName.getQualifier())).build();
}

public static HBaseProtos.RegionInfo toProtoRegionInfo(
org.apache.hadoop.hbase.client.RegionInfo regionInfo) {
return HBaseProtos.RegionInfo.newBuilder()
.setRegionId(regionInfo.getRegionId())
.setRegionEncodedName(regionInfo.getEncodedName())
.setTableName(toProtoTableName(regionInfo.getTable())).build();
}

public static List<TableName> toTableNameList(List<HBaseProtos.TableName> tableNamesList) {
if (tableNamesList == null) {
return new ArrayList<>();
Expand Down Expand Up @@ -3184,6 +3192,9 @@ public static org.apache.hadoop.hbase.client.RegionInfo toRegionInfo(final HBase
if (proto.hasOffline()) {
rib.setOffline(proto.getOffline());
}
if (proto.hasRegionEncodedName()) {
rib.setEncodedName(proto.getRegionEncodedName());
}
return rib.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import org.apache.hadoop.hbase.exceptions.DeserializationException;
import org.apache.hadoop.hbase.filter.ByteArrayComparable;
import org.apache.hadoop.hbase.io.TimeRange;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
import org.apache.hadoop.hbase.replication.SyncReplicationState;
import org.apache.hadoop.hbase.util.Bytes;
Expand Down Expand Up @@ -137,6 +138,7 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.SetBalancerRunningRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.SetCleanerChoreRunningRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.SetNormalizerRunningRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.SetRegionStateInMetaRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos
.SetSnapshotCleanupRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.SetSplitOrMergeEnabledRequest;
Expand Down Expand Up @@ -1444,6 +1446,17 @@ public static SetTableStateInMetaRequest buildSetTableStateInMetaRequest(final T
.setTableName(ProtobufUtil.toProtoTableName(state.getTableName())).build();
}

/**
* Creates a protocol buffer SetRegionStateInMetaRequest
* @param state region state to update in Meta
* @return a SetRegionStateInMetaRequest
*/
public static SetRegionStateInMetaRequest buildSetRegionStateInMetaRequest(
final RegionState state) {
return SetRegionStateInMetaRequest.newBuilder().setRegionState(state.convert())
.setRegionInfo(ProtobufUtil.toProtoRegionInfo(state.getRegion())).build();
}

/**
* Creates a protocol buffer GetTableDescriptorsRequest for a single table
*
Expand Down
1 change: 1 addition & 0 deletions hbase-protocol-shaded/src/main/protobuf/HBase.proto
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ message RegionInfo {
optional bool offline = 5;
optional bool split = 6;
optional int32 replica_id = 7 [default = 0];
optional string region_encoded_name = 8;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is odd... That we didn't have this already?

}

/**
Expand Down
13 changes: 13 additions & 0 deletions hbase-protocol-shaded/src/main/protobuf/Master.proto
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,10 @@ message GetTableStateResponse {
required TableState table_state = 1;
}

message GetRegionStateResponse {
required RegionState region_state = 1;
}


message GetClusterStatusRequest {
repeated Option options = 1;
Expand Down Expand Up @@ -1090,6 +1094,11 @@ message SetTableStateInMetaRequest {
required TableState table_state = 2;
}

message SetRegionStateInMetaRequest {
required RegionInfo region_info = 1;
required RegionState region_state = 2;
}

/** Like Admin's AssignRegionRequest except it can
* take one or more Regions at a time.
*/
Expand Down Expand Up @@ -1152,6 +1161,10 @@ service HbckService {
rpc SetTableStateInMeta(SetTableStateInMetaRequest)
returns(GetTableStateResponse);

/** Update state of the table in meta only*/
rpc SetRegionStateInMeta(SetRegionStateInMetaRequest)
returns(GetRegionStateResponse);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this pattern setting table state where there we reused a GetTableStateResponse as return from SetTableState method. Usually the reponse has same prefix as request -- i.e. the name of the method. I suppose this is ok. Maybe one day we'll have a method that just queries the method state and when that is added, we'll need this GetRegionStateResponse again.

Just noting that this is breaking the general pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, I actually just used SetTableStateInMeta as my template here. I can fix it to be consistent with the method name. Also just noticed a copy&paste mistake in the comment. Will correct that as well on the next commit.


/**
* Assign regions.
* Like Admin's assign but works even if the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.apache.hadoop.hbase.UnknownRegionException;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.MasterSwitchType;
import org.apache.hadoop.hbase.client.Put;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.client.Table;
Expand Down Expand Up @@ -2465,6 +2466,39 @@ public GetTableStateResponse setTableStateInMeta(RpcController controller,
}
}

/**
* Update state of the region in meta only. This is required by hbck in some situations to cleanup
* stuck assign/ unassign regions procedures for the table.
*
* @return previous state of the region
*/
@Override
public MasterProtos.GetRegionStateResponse setRegionStateInMeta(RpcController controller,
MasterProtos.SetRegionStateInMetaRequest request) throws ServiceException {
try {
RegionInfo info = this.master.getAssignmentManager().
loadRegionFromMeta(request.getRegionInfo().getRegionEncodedName());
LOG.trace("region info loaded from meta table: {}", info);
RegionState prevState = this.master.getAssignmentManager().getRegionStates().
getRegionState(info);
RegionState newState = RegionState.convert(request.getRegionState());
LOG.info("{} set region={} state from {} to {}", master.getClientIdAuditPrefix(),
info, prevState.getState(), newState.getState());
Put metaPut = MetaTableAccessor.makePutFromRegionInfo(info, System.currentTimeMillis());
metaPut.addColumn(HConstants.CATALOG_FAMILY,
HConstants.STATE_QUALIFIER, Bytes.toBytes(newState.getState().name()));
List<Put> putList = new ArrayList<>();
putList.add(metaPut);
MetaTableAccessor.putsToMetaTable(this.master.getConnection(), putList);
//Loads from meta again to refresh AM cache with the new region state
this.master.getAssignmentManager().loadRegionFromMeta(info.getEncodedName());
return MasterProtos.GetRegionStateResponse.newBuilder().
setRegionState(prevState.convert()).build();
} catch (Exception e) {
throw new ServiceException(e);
}
saintstack marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Get RegionInfo from Master using content of RegionSpecifier as key.
* @return RegionInfo found by decoding <code>rs</code> or null if none found
Expand Down Expand Up @@ -2834,4 +2868,5 @@ private boolean shouldSubmitSCP(ServerName serverName) {
}
return true;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.hadoop.hbase.coprocessor.ObserverContext;
import org.apache.hadoop.hbase.master.HMaster;
import org.apache.hadoop.hbase.master.RegionState;
import org.apache.hadoop.hbase.master.assignment.AssignmentManager;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
import org.apache.hadoop.hbase.master.procedure.TableProcedureInterface;
import org.apache.hadoop.hbase.procedure2.Procedure;
Expand Down Expand Up @@ -182,6 +183,23 @@ public void testSetTableStateInMeta() throws Exception {
prevState.isDisabled());
}

@Test
public void testSetRegionStateInMEta() throws Exception {
wchevreuil marked this conversation as resolved.
Show resolved Hide resolved
Hbck hbck = getHbck();
try(Admin admin = TEST_UTIL.getAdmin()){
RegionInfo region = admin.getRegions(TABLE_NAME).get(0);
AssignmentManager am = TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager();
RegionState prevState = am.getRegionStates().getRegionState(region);
RegionState newState = RegionState.createForTesting(region,
RegionState.State.CLOSED);
RegionState result = hbck.setRegionStateInMeta(newState);
assertEquals(prevState.getState(), result.getState());
RegionState cachedState = am.getRegionStates().getRegionState(region.getEncodedName());
assertEquals(newState.getState(), cachedState.getState());
hbck.setRegionStateInMeta(prevState);
}
}

@Test
public void testAssigns() throws Exception {
Hbck hbck = getHbck();
Expand Down