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

Make version service return unavailable on standby masters #16854

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

elega
Copy link
Contributor

@elega elega commented Feb 8, 2023

What changes are proposed in this pull request?

Make version service return unavailable on standby masters

Why are the changes needed?

In this #16839 PR, we adds the capability to run grpc services on standby masters. However, if one uses an old alluxio client and connects to the new master with standby master enabled, it will get some errors. This PR is used to address the issue to make the change backward compatible.

Does this PR introduce any user facing changes?

N/A

@qian0817
Copy link
Contributor

overall LGTM. Should we revert the change about PollingMasterInquireClient.java in #16839?

Copy link
Contributor

@jiacheliu3 jiacheliu3 left a comment

Choose a reason for hiding this comment

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

LGTM with a minor comment, thanks!

}

@Override
@SuppressFBWarnings(value = "DB_DUPLICATE_SWITCH_CLAUSES")
public void getServiceVersion(GetServiceVersionPRequest request,
StreamObserver<GetServiceVersionPResponse> responseObserver) {
if (mStandbyRpcEnabled
&& mNodeStateSupplier != null && mNodeStateSupplier.get() == NodeState.STANDBY) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: what does mNodeStateSupplier == null imply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the grpc service is on worker

@elega
Copy link
Contributor Author

elega commented Feb 13, 2023

overall LGTM. Should we revert the change about PollingMasterInquireClient.java in #16839?

Let's keep it as is. Adding this does not do harm.

@jiacheliu3
Copy link
Contributor

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 7399c38 into Alluxio:master Feb 13, 2023
alluxio-bot pushed a commit that referenced this pull request Feb 16, 2023
### 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
#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 #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: #16890
change-id: cid-379f8af78250d2a230bceff9d7ea75739979e198
@Xenorith Xenorith added the area-master Alluxio master label Feb 28, 2023
bzheng888 pushed a commit to bzheng888/alluxio that referenced this pull request Mar 8, 2023
### What changes are proposed in this pull request?

Make version service return unavailable on standby masters

### Why are the changes needed?

In this Alluxio#16839 PR, we adds the
capability to run grpc services on standby masters. However, if one uses
an old alluxio client and connects to the new master with standby master
enabled, it will get some errors. This PR is used to address the issue
to make the change backward compatible.

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

N/A

pr-link: Alluxio#16854
change-id: cid-6c82bce1b1d6cb2649658ec09f0f8ed4e243917a

(cherry picked from commit 7399c38)
bzheng888 pushed a commit to bzheng888/alluxio that referenced this pull request Mar 8, 2023
### 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
(cherry picked from commit da6abf7)
YangchenYe323 pushed a commit to YangchenYe323/alluxio that referenced this pull request Apr 16, 2023
### 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
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 16, 2023
### What changes are proposed in this pull request?

Make version service return unavailable on standby masters

### Why are the changes needed?

In this Alluxio#16839 PR, we adds the
capability to run grpc services on standby masters. However, if one uses
an old alluxio client and connects to the new master with standby master
enabled, it will get some errors. This PR is used to address the issue
to make the change backward compatible.

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

N/A

pr-link: Alluxio#16854
change-id: cid-6c82bce1b1d6cb2649658ec09f0f8ed4e243917a
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 16, 2023
### 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
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 16, 2023
### What changes are proposed in this pull request?

Make version service return unavailable on standby masters

### Why are the changes needed?

In this Alluxio#16839 PR, we adds the
capability to run grpc services on standby masters. However, if one uses
an old alluxio client and connects to the new master with standby master
enabled, it will get some errors. This PR is used to address the issue
to make the change backward compatible.

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

N/A

pr-link: Alluxio#16854
change-id: cid-6c82bce1b1d6cb2649658ec09f0f8ed4e243917a
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 16, 2023
### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-master Alluxio master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants