grpc-go
grpc-go copied to clipboard
balancer: stop using address.Metadata, use address.Attributes instead
address.Metadata is deprecated and replaced by address.Attributes.
The current usage of address.Metadata in gRPC includes:
- grpclb, to pass rpc Metadata
- weighted round robin, to pass weights
This issue is to track removing of these usages, and use address.Attributes instead.
cc @alazarev FYI the way to pass address weights in weighted-round-robin is changing. The weights will be in attributes, instead of metadata, see the change here The plan is to keep both for one release (v1.30.0), and to remove setting address.Metadata later (in v1.31.0)
FYI, the new formulation, a pointer *attributes.Attributes, means that the base balancer will now consider every address a "new address", and recycle every single connection, if the two addresses don't have the same pointer address for attributes. This seems onerous on resolver impls, to have to track an "intern"ed set of attributes for all permutations of attribute values, just so the pointer values can be stable and not cause tons of churn in connections.
This is because it uses resolver.Address as a map key for uniqueness, so an otherwise identical address but with different attributes pointer will be a distinct key.
Thanks for pointing this out, @jhump. We had discussed this awhile back, and knew it would be an issue, but I think we must have forgotten about it when we got around to implementing things. We will need to put together a solution for this ASAP as the use of Attributes is expected to expand.
@dfawley, thanks for the reply. I filed #3611 for this.
The issue of making attributes.Attributes comparable is tracked under #3611.
We have fixed the WRR implementation to pass weights through attributes. But it still also passes the same through metadata. This can be removed based on [comment](https://github.com/grpc/grpc-go/issues/3563#issuecomment-618680599) which says that we can remove it in v1.31.0`.
The other item is to see if we want any changes in grpclb as this is mentioned in comment
@easwars I can't seem to find any more instances of address.Metadata in the codebase. Should we close this?
I do see usages in the ringhash balancer. https://github.com/grpc/grpc-go/blob/db79903af928ca8307700d83123a7702881c4e3c/xds/internal/balancer/ringhash/ring.go#L123
@menghanl : Was this intentional? Can we use the attributes field here?
Also, we should probably investigate if we can completely get rid of the field from resolver.Address and delete existing references here and here. @dfawley
I believe the only place left that we use this internally is a read in the transport to provide the legacy behavior of setting outgoing headers based on it. We never set it. It should be okay to remove now, though it would be a breaking (deprecated + experimental) API change.