search

grpc / grpc-go

The Go language implementation of gRPC. HTTP/2 based RPC
https://grpc.io
Apache License 2.0
21.19k stars 4.41k forks source link

grpc.WithBalancerName("grpclb") is broken #2778

Closed wjywbs closed 5 years ago

wjywbs commented 5 years ago

What version of gRPC are you using?

v1.20.0

What version of Go are you using (go version)?

1.11

What operating system (Linux, Windows, …) and version?

Linux

What did you do?

If I pass grpc.WithBalancerName("grpclb") explicitly in grpc.Dial(), the "grpclb: no remote balancer address is available, should never happen" error is reported.

The curBalancerName is only set when ClientConn.switchBalancer() is called. When balancerBuilder is not nil, switchBalancer is not called and curBalancerName is not set to grpclb.

https://github.com/grpc/grpc-go/blob/v1.20.0/balancer_conn_wrappers.go#L182 https://github.com/grpc/grpc-go/blob/v1.20.0/clientconn.go#L517

What did you expect to see?

RPC call is successful.

What did you see instead?

ERROR: grpclb: no remote balancer address is available, should never happen

war-turtle commented 5 years ago

I would like to solve this issue.

lyuxuan commented 5 years ago

Apology for the delay.

So grpclb is different from other balancers in that it is started when the addresses returned by the resolver has AddressType GRPCLB. There are two sides of it: 1. grpclb won't be started even if you specify you want grpclb (like what you did) if the addresses returned do not contain GRPCLB type address. 2. Even if you specify some other balancer, but your addresses returned by the resolver returned GRPCLB type address, then grpc will still start the grpclb.

May I know why are you using grpclb, and what you want it to do?

Also, FYI, grpclb is deprecated and we are moving to a Enovy xDS protocol based implementation. See more info here. Please let us know what we can help if you need to migrate. Thanks!

wjywbs commented 5 years ago

I'm trying to notify the clients when the backend addresses change and react faster than polling dns. I specified to use grpclb and the resolved addresses contained a grpclb type address, but the error was still reported.

After reading the Envoy xDS protocol, I don't know which service I should implement to provide LoadBalanceResponse's ServerList like response, because I currently don't plan to use Envoy. Being able to provide a delta list of added or removed backend addresses will be preferable.

lyuxuan commented 5 years ago

Let me explain what is grpclb a little bit, it may clear some questions here:

The resolver can return two types of address: Backend and GRPCLB (for grpclb remote server). If grpc notices that at least one of the addresses returned by the resolver is GRPCLB type, it will start grpclb and pass the addresses. Then inside grpclb, it will try to connect to the address(es) with GRPCLB type. Once the connection is set up, the remote grpclb server should send the backend addresses to connect to.

So if you want to notify the clients when the backend addresses change, you first need to implement a grpclb server which will send the backend address update. Then you run this grpclb server somewhere and put its address in the DNS record for the target. Once you done these, grpclb should work for you.

However, there's no open-sourced grpclb server implementation, so you need to implement yourself, which is substantial work.

Some more info: If any connection fails, we will trigger a ResolveNow operation which leads to a DNS resolution. Does that work for you? We think it will be sufficient for most cases.

As for Envoy xDS protocol, you need to implement the EDS service. I can provide more details if you would like to go through this route. Note that xDS is being actively developed right now, so expect potential changes. Thanks!

wjywbs commented 5 years ago

Thanks for your reply. I implemented a prototype grpclb server and a custom dns resolver that returns a grpclb type address. After removing grpc.WithBalancerName("grpclb"), the code path works.

However, grpclb defaults to pick first while I'm trying to switch to round robin. When a backend restarts or more backends are started, dns resolver doesn't react as fast as grpclb, and a few backends may receive no workload for a while.

I didn't find out how to send an incremental list of added or removed backed addresses in grpclb. Streaming the full list to the clients every time when a few backends change is not ideal.

I'd prefer to wait for the Envoy protocol to become stable if grpclb is deprecated.

wjywbs commented 5 years ago

A side question: which method does Google use internally for grpc to connect to GSLB? I'm considering to fork the grpclb balancer if it's no longer maintained by calling balancer.Register(), but I'm not sure about maintaining grpclb in grpc c++ as well.

lyuxuan commented 5 years ago

Internally, we will migrate from grpclb to xds for GSLB. Now, we are still actively maintaining the grpclb balancer, however, we probably won't implement any more feature for it (i.e. new features will most likely be implemented in xds only). So I don't think you need to fork at this point of time. If in the future we decide to remove support for grpclb, we will give an early notice and support for people to migrate smoothly.

As for incremental list, unfortunately, grpclb doesn't support that. Thanks!

dfawley commented 5 years ago

This is resolved by #2802. But in general, it is not intended to pass grpc.WithBalancerName("grpclb") to Dial directly, but instead rely upon the resolver's addresses to indicate it should be chosen, with the backup LB policy selected via service config.