Skip to content

Issue about load balance of gRPC#4251

Closed
solid-itec wants to merge 4 commits into
apache:masterfrom
solid-itec:patch-3
Closed

Issue about load balance of gRPC#4251
solid-itec wants to merge 4 commits into
apache:masterfrom
solid-itec:patch-3

Conversation

@solid-itec
Copy link
Copy Markdown

Fixed issue about loading balance of gRPC on kubenetes via this line of code. If the clients of AGENT deploy on kubernetes and call the OAP service by default cluster IP mode without any technology of service mesh like istio etc. it will occur the issue about traffic load. So for we have resolved this issue via the headless service difination without the clust IP setting and registered multiple A records of DNS for the OAP service on that. Anyway it needs to enhance the code of this class for the defualt behavior of load balance and almost has not any impact if it is out of kubernetes environment. Seems to work smoothly.

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

wu-sheng and others added 4 commits December 24, 2019 06:07
Fixed issue about loading balance of gRPC on kubenetes via this line of code. If the clients of AGENT deploy on kubernetes and call the OAP service by default cluster IP mode without any technology of service mesh like istio etc. it will occur the issue about traffic load. So for we have resolved this issue via the headless service difination without the clust IP setting and registered multiple A records of DNS for the OAP service on that. Anyway it needs to enhance the code of this class for the defualt behavior of load balance and almost has not any impact if it is out of kubernetes environment. Seems to work smoothly.
@wu-sheng wu-sheng added this to the 7.0.0 milestone Jan 16, 2020
@wu-sheng wu-sheng added agent Language agent related. feature New feature labels Jan 16, 2020
@Override public ManagedChannelBuilder build(ManagedChannelBuilder managedChannelBuilder) throws Exception {
return managedChannelBuilder.nameResolverFactory(new DnsNameResolverProvider())
/**
* It seems the code above can only work for low gRPC version.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What does this comment mean?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't need to describe the history.

/**
* It seems the code above can only work for low gRPC version.
*/
.loadBalancerFactory(RoundRobinLoadBalancerFactory.getInstance())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

According to the header of RoundRobinLoadBalancerFactory, this is experimental. And also, this could use more connections of OAP backend, you should create a Config, at Config#Collector#OPEN_ROUND_ROBIN_LOAD_BALANCE default false, to support this feature.

/**
 * A {@link LoadBalancer} that provides round-robin load balancing mechanism over the
 * addresses from the {@link NameResolver}.  The sub-lists received from the name resolver
 * are considered to be an {@link EquivalentAddressGroup} and each of these sub-lists is
 * what is then balanced across.
 */
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1771")
public class RoundRobinLoadBalancerFactory extends LoadBalancer.Factory {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

OK. Will add these details ASAP

@kezhenxu94
Copy link
Copy Markdown
Member

kezhenxu94 commented Jan 16, 2020

@wu-sheng I guess there're something wrong with the GitHub settings, the required checks are failed and no approvals from committers, the merge button still shows, do you know why it's the case?

image

@wu-sheng
Copy link
Copy Markdown
Member

@kezhenxu84 I feel strange too, just created this, https://issues.apache.org/jira/browse/INFRA-19734

@wu-sheng
Copy link
Copy Markdown
Member

@kezhenxu94 Strange, #4220 shows checks are existing.

@wu-sheng wu-sheng closed this Jan 16, 2020
@wu-sheng wu-sheng reopened this Jan 16, 2020
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Request improvements as comments above.

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #4251 into 6.x will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             6.x    #4251      +/-   ##
=========================================
- Coverage   27.3%   27.24%   -0.06%     
=========================================
  Files       1140     1140              
  Lines      25027    25028       +1     
  Branches    3619     3619              
=========================================
- Hits        6833     6819      -14     
- Misses     17588    17608      +20     
+ Partials     606      601       -5
Impacted Files Coverage Δ
.../apm/agent/core/remote/StandardChannelBuilder.java 100% <100%> (ø) ⬆️
...apm/agent/core/remote/GRPCStreamServiceStatus.java 29.16% <0%> (-25.01%) ⬇️
.../core/remote/ServiceAndEndpointRegisterClient.java 31.46% <0%> (-13.49%) ⬇️
...alking/apm/agent/core/logging/core/FileWriter.java 82.02% <0%> (-2.25%) ⬇️
.../agent/core/context/trace/AbstractTracingSpan.java 60.36% <0%> (-0.91%) ⬇️
...lking/apm/agent/core/sampling/SamplingService.java 75.75% <0%> (+3.03%) ⬆️
...king/apm/agent/core/remote/GRPCChannelManager.java 67.94% <0%> (+3.84%) ⬆️
...alking/apm/agent/core/commands/CommandService.java 40.42% <0%> (+4.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffbe9f5...3ca9fc0. Read the comment docs.

@wu-sheng
Copy link
Copy Markdown
Member

This PR is targeting the wrong branch, so this happens. @bennyparlo You should send a pull request to the master branch, as this is a feature change. 6.x is in maintenance mode only.

@wu-sheng wu-sheng closed this Jan 16, 2020
@kezhenxu94 kezhenxu94 reopened this Jan 16, 2020
@kezhenxu94 kezhenxu94 changed the base branch from 6.x to master January 16, 2020 13:46
@kezhenxu94 kezhenxu94 closed this Jan 16, 2020
@solid-itec solid-itec deleted the patch-3 branch January 17, 2020 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Language agent related. feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants