Skip to content

api,core: add LoadBalancer.Helper#createResolvingOobChannelBuilder api - #7136

Merged
creamsoup merged 2 commits into
grpc:masterfrom
creamsoup:resolving_channel_builder
Jun 17, 2020
Merged

api,core: add LoadBalancer.Helper#createResolvingOobChannelBuilder api#7136
creamsoup merged 2 commits into
grpc:masterfrom
creamsoup:resolving_channel_builder

Conversation

@creamsoup

Copy link
Copy Markdown
Contributor

this allows RLS or any LB to configure OobChannel such as service config.

Comment thread core/src/main/java/io/grpc/internal/ManagedChannelImpl.java Outdated
* target string. The out-of-band channel doesn't have load-balancing. If multiple addresses
* are resolved for the target, the first working address will be used.
* target string. If multiple addresses are resolved for the target, the first working address
* will be used.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the last sentence is not needed as well. Basically it will use the balancer determined by the service config from the NameResolver or the default config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, removed.

}

@Override
public ManagedChannel createResolvingOobChannel(String target) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you still need this overriding? Same for below.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For below we definitely need.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. for the Forwarding one, i didn't change because the method is not final.

@dapengzhang0 dapengzhang0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@creamsoup
creamsoup merged commit f5258dc into grpc:master Jun 17, 2020
@creamsoup
creamsoup deleted the resolving_channel_builder branch June 17, 2020 19:36
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants