Skip to content
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

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

Closed
wjywbs opened this issue Apr 18, 2019 · 8 comments
Assignees
Labels

Comments

@wjywbs
Copy link
Contributor

@wjywbs wjywbs commented Apr 18, 2019

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

This comment has been minimized.

Copy link
Contributor

@war-turtle war-turtle commented Apr 20, 2019

I would like to solve this issue.

@lyuxuan

This comment has been minimized.

Copy link
Contributor

@lyuxuan lyuxuan commented Apr 22, 2019

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

This comment has been minimized.

Copy link
Contributor Author

@wjywbs wjywbs commented Apr 22, 2019

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

This comment has been minimized.

Copy link
Contributor

@lyuxuan lyuxuan commented Apr 22, 2019

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

This comment has been minimized.

Copy link
Contributor Author

@wjywbs wjywbs commented Apr 23, 2019

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

This comment has been minimized.

Copy link
Contributor Author

@wjywbs wjywbs commented Apr 23, 2019

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

This comment has been minimized.

Copy link
Contributor

@lyuxuan lyuxuan commented May 2, 2019

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

This comment has been minimized.

Copy link
Contributor

@dfawley dfawley commented May 10, 2019

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.

@dfawley dfawley closed this May 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.