balancer: start populating weight by edsbalancer for weighted_round_robin #2945
Conversation
…weight in metadata version). Weight is stored as wrr.Info in Address metadata.
balancer/wrr/wrr.go
Outdated
| const Name = "weighted_round_robin" | ||
|
|
||
| // Information that should be stored inside Address metadata in order to use wrr. | ||
| type Info struct{ |
There was a problem hiding this comment.
Maybe let's make it an interface, e.g.
type Weighted interface {
Weight() uint32
}
?
This would allow us to provide some object with xds-specific knowledge (e.g. is it a canary instance?) in the future, which would also implement the Weighted interface among others.
There was a problem hiding this comment.
Also it may make sense to move this interface to the balancer package itself, as it's possible it would be used by other weight-aware load balancers, e.g. maglev.
There was a problem hiding this comment.
The problem with interface is, it's hard to add methods. But for structs, we can add fields.
I also think it's better to not keep it in balancer package, because it's more like a feature of a specific balancer, not all balancers.
balancer/base/balancer.go
Outdated
| @@ -60,7 +61,7 @@ type baseBalancer struct { | |||
| csEvltr *balancer.ConnectivityStateEvaluator | |||
| state connectivity.State | |||
|
|
|||
| subConns map[resolver.Address]balancer.SubConn | |||
| subConns map[resolver.Address]AddrInfo | |||
There was a problem hiding this comment.
With changes in this file, we will still unnecessarily reconnect when the weight for an address changes.
To avoid that, we should clear metadata field when addresses are received.
But this changes the behavior.
Base is to be used as an example, and in the case only picking algorithm is different.
I think we should leave base package as is, and re-implement this for wrr.
wrr will have some special behaviors, some I can think of now:
- Duplicates in addresses list (same address string, but multiple entries in addrs list, with potential different metadata)
- Should we add up the weights?
- Since metadata is not hashed, the only useful thing is the address string itself. Maybe we should use the string as the key, instead of the address struct
There was a problem hiding this comment.
In this care diff becomes really small, just weightedroundrobin package declaration with metadata population in edsbalancer. Updated pull request.
Reverted balancer/base/balancer.go Renamed wrr.Info to weightedroundrobin.AddrInfo
|
Updated |
alazarev
changed the title
|
Updated title to match reality |
menghanl
changed the title
|
@menghanl Do you have an idea what continuous-integration/travis-ci/pr is complaining about? It shows no errors, but build is marked as failed. |
|
Updated. Looks like we are ready to merge. |
Weight is stored as wrr.Info in Address metadata.