A119: Auto-Sharding LB Policy - #551
Conversation
|
@markdroth @ejona86 @dfawley |
|
I'm going to be making some structural changes to the spec based on our review today. Will ping here once it is ready for review. @markdroth @ejona86 @dfawley @shivaspeaks @pawbhard Please expect some changes to the spec, but not wholesale changes. So, you could start looking at it from an implementation pov. Thanks. |
|
The PR is ready to be looked at. |
markdroth
left a comment
markdroth
left a comment
There was a problem hiding this comment.
This looks really good!
I have a lot of comments here, but they're mostly fine details or clarifications -- the high-level shape of this is solid, and the doc does a good job of describing it.
Please let me know if you have any questions. Thanks!
markdroth
left a comment
markdroth
left a comment
There was a problem hiding this comment.
This is getting closer!
Please let me know if you have any questions. Thanks!
| // A timeout value for fallback to kick in when no assignments have been | ||
| // received from the sharding service. | ||
| // Defaults to 60 seconds if not specified. | ||
| google.protobuf.Duration fallback_timeout = 6; |
There was a problem hiding this comment.
It doesn't really matter whether it's defined here or in A121. I guess it should be in whichever of the two gRFCs is going to be merged last.
Please coordinate with @mbissa.
| allEndpointsInSlice []int // Indices into sliceMap.allEndpoints | ||
| endpointsByState [5][]int // Array indexed directly by connectivity.State (ranges 0..4) |
There was a problem hiding this comment.
Okay. @pawbhard, let's consider this for the C-core implementation.
| 1. Parsing a `GrpcService` proto embedded within an LB policy's configuration | ||
| into its internal representation, requires access to the following: | ||
| * the complete bootstrap configuration to access the `allowed_grpc_services` | ||
| section of the bootstrap configuration. | ||
| * configuration of the specific xDS server that delivered this resource, to | ||
| determine if the server is to be trusted or not. |
|
@markdroth : I think I've addressed all your comments. Thanks for the detailed review. |
| If validation fails, the LB policy must terminate the stream to the sharding | ||
| service, and attempt to re-establish it. |
There was a problem hiding this comment.
Is this really the right thing to do? If we get an invalid assignment, that's likely a bug in the slicer service, and it's not clear to me that reestablishing the stream will fix it -- we'll probably just get the same bad data again after we restart the stream. But restarting the stream will cause load on the server, so I'd be worried about a cascading failure here: a bug in the slicer service could cause all of the clients to start terminating and reconnecting streams, which could cause a lot of unexpected load.
I suggest asking the slicer folks how they handle this kind of case today.
4 participants
No description provided.