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

feat: adaptive load balancer of adaptive service #2067

Merged
merged 18 commits into from Dec 10, 2022

Conversation

CoolIceV
Copy link
Contributor

@CoolIceV CoolIceV commented Oct 2, 2022

What this PR does:
Provides an adaptive load balancer in unstable environments.
This is a component of adaptive service at the consumer side.

Which issue(s) this PR fixes:
Fixes #1834

You should pay attention to items below to ensure your pr passes our ci test
We do not merge pr with ci tests failed

  • All ut passed (run 'go test ./...' in project root)
  • After go-fmt ed , run 'go fmt project' using goland.
  • Golangci-lint passed, run 'sudo golangci-lint run' in project root.
  • Your new-created file needs to have apache license at the top, like other existed file does.
  • All integration test passed. You can run integration test locally (with docker env). Clone our dubbo-go-samples project and replace the go.mod to your dubbo-go, and run 'sudo sh start_integration_test.sh' at root of samples project root. (M1 Slice is not Support)

@CoolIceV
Copy link
Contributor Author

CoolIceV commented Oct 2, 2022

This load balancer takes into account the recent request success rate and request latency, the effect is as follows.

Flexible services are turned on at the marked line position. After that the QPS of different configured services is no longer the same, and the CPU load and latency of lower configured services are significantly reduced.

qps

cpu

rtt99

@CoolIceV CoolIceV marked this pull request as ready for review October 2, 2022 04:52
@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2022

Codecov Report

Merging #2067 (11b348b) into 3.0 (19ddecf) will increase coverage by 0.05%.
The diff coverage is 93.10%.

❗ Current head 11b348b differs from pull request most recent head 98e894a. Consider uploading reports for the commit 98e894a to get more accurate results

@@            Coverage Diff             @@
##              3.0    #2067      +/-   ##
==========================================
+ Coverage   44.60%   44.66%   +0.05%     
==========================================
  Files         283      281       -2     
  Lines       17105    16841     -264     
==========================================
- Hits         7630     7522     -108     
+ Misses       8669     8532     -137     
+ Partials      806      787      -19     
Impacted Files Coverage Δ
cluster/loadbalance/p2c/loadbalance.go 96.66% <93.10%> (+16.66%) ⬆️
metadata/identifier/base_metadata_identifier.go 86.48% <0.00%> (-7.64%) ⬇️
...tocol/rest/server/server_impl/go_restful_server.go 41.37% <0.00%> (-3.45%) ⬇️
xds/utils/credentials/xds/handshake_info.go 41.46% <0.00%> (ø)
...y/event/service_instances_changed_listener_impl.go 0.00% <0.00%> (ø)
remoting/xds/client.go
remoting/xds/xds_client_factory.go
common/url.go 59.50% <0.00%> (+0.45%) ⬆️
config/logger_config.go 63.33% <0.00%> (+0.62%) ⬆️
registry/nacos/registry.go 46.00% <0.00%> (+2.68%) ⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more


// EMA is a struct implemented Exponential Moving Average.
// val = old * (1 - alpha) + new * alpha
type EMA struct {
Copy link
Member

Choose a reason for hiding this comment

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

"EMA" and "SlidingWindowCounter" as metrics name might make others confused. Please give your approach a name, such as "hill climbing" (just an example), and move all of them to <Name>Metrics. If it will be extended to more approaches in the future, then they will be named <Name1>Metrics, <Name2>Metrics, etc.

Copy link
Member

Choose a reason for hiding this comment

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

To make it clearer, please keep EMA and SlidingWindowCounter in each file, create a new file, named <Name>_metrics.go, then merge EMAMetrics and SlidingWindowCounterMetrics into <Name>Metrics.

return c.size
}

func (c *SlidingWindowCounter) Add(_ int64) {
Copy link
Member

Choose a reason for hiding this comment

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

Can this parameter be deleted?

c.count++
}

func (c *SlidingWindowCounter) Value() int64 {
Copy link
Member

Choose a reason for hiding this comment

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

Value is not a thread-safe method.

Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Is this PR for client? Will traffic limitation on the server side be submitted in another PR?

* limitations under the License.
*/

package rolling
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this file to sliding_window_counter_metrics.go.

@justxuewei
Copy link
Member

Lgtm, thanks.

@CoolIceV
Copy link
Contributor Author

Is this PR for client? Will traffic limitation on the server side be submitted in another PR?

Yes.

@CoolIceV CoolIceV changed the base branch from 3.0 to 3.0-adaptive-stream November 24, 2022 11:55
@sonarcloud
Copy link

sonarcloud bot commented Nov 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@AlexStocks AlexStocks merged commit 4ed7c71 into apache:3.0-adaptive-stream Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants