Skip to content

HBASE-23304: RPCs needed for client meta information lookup #904

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 19, 2019
Merged

HBASE-23304: RPCs needed for client meta information lookup #904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c0a43e
HBASE-23304: RPCs needed for client meta information lookup
bharathv Nov 21, 2019
6fd9ac8
Fix test type annotation.
bharathv Dec 11, 2019
5e3dcc4
Address Andrew's comment
bharathv Dec 19, 2019
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
4 changes: 3 additions & 1 deletion hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,9 @@ private static IOException makeIOExceptionOfException(Throwable e) {
* @see #toServerName(org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ServerName)
*/
public static HBaseProtos.ServerName toServerName(final ServerName serverName) {
if (serverName == null) return null;
if (serverName == null) {
return null;
}
HBaseProtos.ServerName.Builder builder =
HBaseProtos.ServerName.newBuilder();
builder.setHostName(serverName.getHostname());
Expand Down
44 changes: 44 additions & 0 deletions hbase-protocol-shaded/src/main/protobuf/Master.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1200,3 +1200,47 @@ service HbckService {
rpc FixMeta(FixMetaRequest)
returns(FixMetaResponse);
}

/** Request and response to get the clusterID for this cluster */
message GetClusterIdRequest {
}
message GetClusterIdResponse {
/** Not set if cluster ID could not be determined. */
optional string cluster_id = 1;
}

/** Request and response to get the currently active master name for this cluster */
message GetActiveMasterRequest {
}
message GetActiveMasterResponse {
/** Not set if an active master could not be determined. */
optional ServerName server_name = 1;
}

/** Request and response to get the current list of meta region locations */
message GetMetaRegionLocationsRequest {
}
message GetMetaRegionLocationsResponse {
/** Not set if meta region locations could not be determined. */
repeated RegionLocation meta_locations = 1;
}

/**
* Implements all the RPCs needed by clients to look up cluster meta information needed for connection establishment.
*/
service ClientMetaService {
Copy link
Member

Choose a reason for hiding this comment

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

Not saying it's right or wrong (yet), but why define a new service rather than add these methods to the existing "MasterService" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just seemed logical to me to group these (and possibly related RPCs that we add in the future) into a separate service that master is just implementing today. Theoretically they are not tied to masters in any way. Any other process can implement it too (like you already mentioned in the jira comments of HBASE-18095). What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of a separate service. If someone wants to move this later.

/**
* Get Cluster ID for this cluster.
*/
rpc GetClusterId(GetClusterIdRequest) returns(GetClusterIdResponse);

/**
* Get active master server name for this cluster.
*/
rpc GetActiveMaster(GetActiveMasterRequest) returns(GetActiveMasterResponse);

/**
* Get current meta replicas' region locations.
*/
rpc GetMetaRegionLocations(GetMetaRegionLocationsRequest) returns(GetMetaRegionLocationsResponse);
}
85 changes: 64 additions & 21 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package org.apache.hadoop.hbase.master;

import static org.apache.hadoop.hbase.master.MasterWalManager.META_FILTER;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.BindException;
Expand All @@ -30,13 +29,15 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.ClusterMetricsBuilder;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.HRegionLocation;
import org.apache.hadoop.hbase.MetaTableAccessor;
import org.apache.hadoop.hbase.NamespaceDescriptor;
import org.apache.hadoop.hbase.Server;
Expand Down Expand Up @@ -116,11 +117,9 @@
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

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

import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
import org.apache.hadoop.hbase.shaded.protobuf.ResponseConverter;
import org.apache.hadoop.hbase.shaded.protobuf.generated.AccessControlProtos;
Expand Down Expand Up @@ -161,6 +160,7 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.BalanceResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ClearDeadServersRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ClearDeadServersResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ClientMetaService;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.CreateNamespaceRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.CreateNamespaceResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.CreateTableRequest;
Expand All @@ -185,12 +185,18 @@
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.ExecProcedureResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FixMetaRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.FixMetaResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetActiveMasterRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetActiveMasterResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterIdRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterIdResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterStatusRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetClusterStatusResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetCompletedSnapshotsRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetCompletedSnapshotsResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetLocksRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetLocksResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetMetaRegionLocationsRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetMetaRegionLocationsResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetNamespaceDescriptorRequest;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetNamespaceDescriptorResponse;
import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetProcedureResultRequest;
Expand Down Expand Up @@ -351,9 +357,10 @@
*/
@InterfaceAudience.Private
@SuppressWarnings("deprecation")
public class MasterRpcServices extends RSRpcServices
implements MasterService.BlockingInterface, RegionServerStatusService.BlockingInterface,
LockService.BlockingInterface, HbckService.BlockingInterface {
public class MasterRpcServices extends RSRpcServices implements
MasterService.BlockingInterface, RegionServerStatusService.BlockingInterface,
LockService.BlockingInterface, HbckService.BlockingInterface,
ClientMetaService.BlockingInterface {
private static final Logger LOG = LoggerFactory.getLogger(MasterRpcServices.class.getName());
private static final Logger AUDITLOG =
LoggerFactory.getLogger("SecurityLogger."+MasterRpcServices.class.getName());
Expand All @@ -362,7 +369,7 @@ public class MasterRpcServices extends RSRpcServices

/**
* @return Subset of configuration to pass initializing regionservers: e.g.
* the filesystem to use and root directory to use.
* the filesystem to use and root directory to use.
*/
private RegionServerStartupResponse.Builder createConfigurationSubset() {
RegionServerStartupResponse.Builder resp = addConfig(
Expand Down Expand Up @@ -488,15 +495,17 @@ boolean synchronousBalanceSwitch(final boolean b) throws IOException {
protected List<BlockingServiceAndInterface> getServices() {
List<BlockingServiceAndInterface> bssi = new ArrayList<>(5);
bssi.add(new BlockingServiceAndInterface(
MasterService.newReflectiveBlockingService(this),
MasterService.BlockingInterface.class));
MasterService.newReflectiveBlockingService(this),
MasterService.BlockingInterface.class));
bssi.add(new BlockingServiceAndInterface(
RegionServerStatusService.newReflectiveBlockingService(this),
RegionServerStatusService.BlockingInterface.class));
RegionServerStatusService.newReflectiveBlockingService(this),
RegionServerStatusService.BlockingInterface.class));
bssi.add(new BlockingServiceAndInterface(LockService.newReflectiveBlockingService(this),
LockService.BlockingInterface.class));
bssi.add(new BlockingServiceAndInterface(HbckService.newReflectiveBlockingService(this),
HbckService.BlockingInterface.class));
bssi.add(new BlockingServiceAndInterface(ClientMetaService.newReflectiveBlockingService(this),
ClientMetaService.BlockingInterface.class));
bssi.addAll(super.getServices());
return bssi;
}
Expand Down Expand Up @@ -623,7 +632,9 @@ public AssignRegionResponse assignRegion(RpcController controller,

final byte[] regionName = req.getRegion().getValue().toByteArray();
final RegionInfo regionInfo = master.getAssignmentManager().getRegionInfo(regionName);
if (regionInfo == null) throw new UnknownRegionException(Bytes.toStringBinary(regionName));
if (regionInfo == null) {
throw new UnknownRegionException(Bytes.toStringBinary(regionName));
}

final AssignRegionResponse arr = AssignRegionResponse.newBuilder().build();
if (master.cpHost != null) {
Expand Down Expand Up @@ -668,7 +679,7 @@ public CreateNamespaceResponse createNamespace(RpcController controller,

@Override
public CreateTableResponse createTable(RpcController controller, CreateTableRequest req)
throws ServiceException {
throws ServiceException {
TableDescriptor tableDescriptor = ProtobufUtil.toTableDescriptor(req.getTableSchema());
byte [][] splitKeys = ProtobufUtil.getSplitKeysArray(req);
try {
Expand Down Expand Up @@ -1062,7 +1073,7 @@ public GetSchemaAlterStatusResponse getSchemaAlterStatus(
* Get list of TableDescriptors for requested tables.
* @param c Unused (set to null).
* @param req GetTableDescriptorsRequest that contains:
* - tableNames: requested tables, or if empty, all are requested
* - tableNames: requested tables, or if empty, all are requested.
* @return GetTableDescriptorsResponse
* @throws ServiceException
*/
Expand Down Expand Up @@ -1206,9 +1217,9 @@ public IsProcedureDoneResponse isProcedureDone(RpcController controller,
/**
* Checks if the specified snapshot is done.
* @return true if the snapshot is in file system ready to use,
* false if the snapshot is in the process of completing
* false if the snapshot is in the process of completing
* @throws ServiceException wrapping UnknownSnapshotException if invalid snapshot, or
* a wrapped HBaseSnapshotException with progress failure reason.
* a wrapped HBaseSnapshotException with progress failure reason.
*/
@Override
public IsSnapshotDoneResponse isSnapshotDone(RpcController controller,
Expand Down Expand Up @@ -1450,7 +1461,9 @@ public OfflineRegionResponse offlineRegion(RpcController controller,

final byte[] regionName = request.getRegion().getValue().toByteArray();
final RegionInfo hri = master.getAssignmentManager().getRegionInfo(regionName);
if (hri == null) throw new UnknownRegionException(Bytes.toStringBinary(regionName));
if (hri == null) {
throw new UnknownRegionException(Bytes.toStringBinary(regionName));
}

if (master.cpHost != null) {
master.cpHost.preRegionOffline(hri);
Expand Down Expand Up @@ -2311,8 +2324,8 @@ public RegionSpaceUseReportResponse reportRegionSpaceUse(RpcController controlle
report.getRegionSize(), now);
}
} else {
LOG.debug(
"Received region space usage report but HMaster is not ready to process it, skipping");
LOG.debug("Received region space usage report but HMaster is not ready to process it, "
+ "skipping");
}
return RegionSpaceUseReportResponse.newBuilder().build();
} catch (Exception e) {
Expand Down Expand Up @@ -2348,8 +2361,8 @@ public GetSpaceQuotaRegionSizesResponse getSpaceQuotaRegionSizes(
}
return builder.build();
} else {
LOG.debug(
"Received space quota region size report but HMaster is not ready to process it, skipping");
LOG.debug("Received space quota region size report but HMaster is not ready to process it,"
+ "skipping");
}
return builder.build();
} catch (Exception e) {
Expand Down Expand Up @@ -2893,4 +2906,34 @@ private boolean shouldSubmitSCP(ServerName serverName) {
return true;
}

@Override
public GetClusterIdResponse getClusterId(RpcController rpcController, GetClusterIdRequest request)
throws ServiceException {
GetClusterIdResponse.Builder resp = GetClusterIdResponse.newBuilder();
String clusterId = master.getClusterId();
if (clusterId != null) {
resp.setClusterId(clusterId);
}
return resp.build();
}

@Override
public GetActiveMasterResponse getActiveMaster(RpcController rpcController,
GetActiveMasterRequest request) throws ServiceException {
GetActiveMasterResponse.Builder resp = GetActiveMasterResponse.newBuilder();
Optional<ServerName> serverName = master.getActiveMaster();
serverName.ifPresent(name -> resp.setServerName(ProtobufUtil.toServerName(name)));
Copy link
Contributor

Choose a reason for hiding this comment

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

These Java 8 constructs don't add much and make backports harder. No issues here, just a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya, will clean this up in the back ports separately.

return resp.build();
}

@Override
public GetMetaRegionLocationsResponse getMetaRegionLocations(RpcController rpcController,
GetMetaRegionLocationsRequest request) throws ServiceException {
GetMetaRegionLocationsResponse.Builder response = GetMetaRegionLocationsResponse.newBuilder();
Optional<List<HRegionLocation>> metaLocations =
master.getMetaRegionLocationCache().getMetaRegionLocations();
metaLocations.ifPresent(hRegionLocations -> hRegionLocations.forEach(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

location -> response.addMetaLocations(ProtobufUtil.toRegionLocation(location))));
return response.build();
}
}
Loading