-
Notifications
You must be signed in to change notification settings - Fork 4k
use an attribute from resolved addresses IS_PETIOLE_POLICY to control whether or not health checking is supported #11513
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
Conversation
… whether or not health checking is supported so that top level versions can't do any health checking, while those under petiole policies can. Fixes grpc#11413
|
The health checking does not need this flag, iirc. |
YifeiZhuang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks this is still needed.
| } | ||
| if (subchannelData.getHealthState() == READY) { | ||
|
|
||
| if (notAPetiolePolicy || subchannelData.getHealthState() == READY) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just always set health status to READY when creating the subchannel. Same as L454.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the if at line 453 to use notAPetiolePolicy for auto setting as ready, but I'd like to leave this here to make it clear to people maintaining the code what is happening since understanding that somewhere else set the value to READY and nowhere else should have changed it is difficult.
| public void onSubchannelState(ConnectivityStateInfo newState) { | ||
| if (notAPetiolePolicy) { | ||
| log.log(Level.WARNING, | ||
| "Ignoring health status {0} for subchannel {1} as this is not a petiole policy", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we say "not a petiole policy" or "there is no petiole policy". This policy is always leaf policy, the difference is whether there is a petiole policy or this is top level policy. Health check can not be used with health producer + pick first (without petiole policy).
A61:
This set of policies, which we will refer to as "petiole" policies, includes the following:
round_robin (see gRPC Load Balancing)
weighted_round_robin (see gRFC A58)
ring_hash (see gRFC A42)
least_request (see gRFC A48 -- currently supported in Java and Go only)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated message to say "not under a petiole policy"
…et health state to READY when creating the subchannel for notAPetiolePolicy.
This blocks top level versions from doing any health checking, while those under petiole policies can.
Fixes #11413