Skip to content

Commit

Permalink
Add allowOnStandbyMasters option for version grpc endpoint
Browse files Browse the repository at this point in the history
### What changes are proposed in this pull request?

Add a rejectOnStandbyMasters on version grpc endpoint

### Why are the changes needed?

We added a PR to make standby master return unavailable on this version
service endpoint
Alluxio#16854

However, in addition to the polling master inquire client, the
AbstractMasterClient also needs this endpoint.

https://github.com/Alluxio/alluxio/blob/master/core/common/src/main/java/alluxio/AbstractClient.java#L172
When a client makes a request to standby master, such check will
constantly fail and resulted in failures.

So we want the logic done in Alluxio#16854 only applies to
PollingMasterInquireClient and hence we added this boolean field to
bypass the check.

### Does this PR introduce any user facing changes?

N/A

pr-link: Alluxio#16890
change-id: cid-379f8af78250d2a230bceff9d7ea75739979e198
  • Loading branch information
elega authored and jiacheliu3 committed May 16, 2023
1 parent a65f8cf commit 0c210d7
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
21 changes: 21 additions & 0 deletions common/transport/src/main/proto/grpc/version.proto
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@ enum ServiceType {

message GetServiceVersionPRequest {
optional ServiceType serviceType = 1;
// The purpose of this field is to make grpc service on standby masters work without
// making client changes and keeps backwards compatibility.
// This requests to this endpoint will be rejected on standby masters by default,
// unless this field is set.
// Two places use this request:
// 1. PollingMasterInquireClient uses this endpoint to tell who is the primary master.
// 2. AbstractClient uses this endpoint to verify the version before it RPCs with the master.
//
// Behaviors:
// 1. old clients -> new cluster standby masters
// PollingMasterInquireClient does not set this field and is able to tell which one is primary master because
// the request will be rejected on the standby master.
// AbstractClient does not set this field.
// Old clients only connects to primary so this doesn't break the existing behavior.
//
// 2. new clients -> new cluster standby masters
// PollingMasterInquireClient does not set this field and is able to tell which one is primary master because
// the request will be rejected on the standby master.
// AbstractClient sets this field to true. Rpcs to standby masters can go through and pass the version verification.

optional bool allowedOnStandbyMasters = 2;
}
message GetServiceVersionPResponse {
optional int64 version = 1;
Expand Down
5 changes: 5 additions & 0 deletions common/transport/src/main/proto/proto.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8487,6 +8487,11 @@
"id": 1,
"name": "serviceType",
"type": "ServiceType"
},
{
"id": 2,
"name": "allowedOnStandbyMasters",
"type": "bool"
}
]
},
Expand Down
5 changes: 4 additions & 1 deletion dora/core/common/src/main/java/alluxio/AbstractClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ protected long getRemoteServiceVersion() throws AlluxioStatusException {
try {
return mVersionService
.getServiceVersion(
GetServiceVersionPRequest.newBuilder().setServiceType(getRemoteServiceType()).build())
GetServiceVersionPRequest.newBuilder()
.setServiceType(getRemoteServiceType())
.setAllowedOnStandbyMasters(true)
.build())
.getVersion();
} catch (Throwable t) {
throw AlluxioStatusException.fromThrowable(t);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ public ServiceVersionClientServiceHandler(
@SuppressFBWarnings(value = "DB_DUPLICATE_SWITCH_CLAUSES")
public void getServiceVersion(GetServiceVersionPRequest request,
StreamObserver<GetServiceVersionPResponse> responseObserver) {
if (mStandbyRpcEnabled
// getAllowedOnStandbyMasters() is defaulted to false
if (!request.getAllowedOnStandbyMasters() && mStandbyRpcEnabled
&& mNodeStateSupplier != null && mNodeStateSupplier.get() == NodeState.STANDBY) {
responseObserver.onError(Status.UNAVAILABLE
.withDescription("GetServiceVersion is not supported on standby master")
Expand Down

0 comments on commit 0c210d7

Please sign in to comment.