fix: qps is not correct #221

ii2day · 2023-09-22T08:44:23Z

原方法：每个并发协程平均分配请求，这样会导致，请求数量大于期望的请求数量，影响结果，如期望发送 15 个请求，10 个并发协程执行，当完成 10 个请求后，剩余 5 个请求被 5 个协程分配，但剩下的 5 个协程没有被分配到依然会去请求，影响期望结果。

现方法：将请求的数量放入一个 channel 中，每个并发协程去channel 中拿请求的许可，拿到许可才进行请求。

codecov · 2023-09-22T08:47:54Z

Codecov Report

Merging #221 (1139931) into main (49beec5) will increase coverage by 0.35%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##             main     #221      +/-   ##
==========================================
+ Coverage   39.16%   39.52%   +0.35%     
==========================================
  Files           8        8              
  Lines         526      544      +18     
==========================================
+ Hits          206      215       +9     
- Misses        315      323       +8     
- Partials        5        6       +1

Flag	Coverage Δ
unittests	`39.52% <77.77%> (+0.35%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
pkg/loadRequest/loadDns/dns.go	`90.90% <100.00%> (-1.20%)`	⬇️
pkg/loadRequest/loadDns/dns_reporter.go	`87.09% <ø> (ø)`
pkg/loadRequest/loadDns/dns_requester.go	`87.58% <76.92%> (-5.04%)`	⬇️

weizhoublue · 2023-09-24T23:53:25Z

我记得当初有要求完成在 server 应用端验证请求数，这个用例没验证出这大bug 么？如果没有，请闭环用例

ii2day · 2023-09-25T02:46:34Z

我记得当初有要求完成在 server 应用端验证请求数，这个用例没验证出这大bug 么？如果没有，请闭环用例

之前的用例是实际请求与期望请求差值在 10%就可以，现在已经更改为实际与期望相等才可通过。

weizhoublue · 2023-09-26T16:06:43Z

pr 描述中， “如期望发送 15 个请求，10 个并发协程执行，当完成 10 个请求后，剩余 5 个请求被 5 个协程分配，但剩下的 5 个协程没有被分配到依然会去请求，影响期望结果”

而之前e2e 偏差容忍是 10%，那 PR 描述不准确，还是 E2E 有 bug 没测试出来

ii2day · 2023-09-27T01:56:54Z

pr 描述中， “如期望发送 15 个请求，10 个并发协程执行，当完成 10 个请求后，剩余 5 个请求被 5 个协程分配，但剩下的 5 个协程没有被分配到依然会去请求，影响期望结果”

而之前e2e 偏差容忍是 10%，那 PR 描述不准确，还是 E2E 有 bug 没测试出来

之前的e2e 设置的 qps 以及并发量正好偏差在 10%以内所以用例通过了，现在已经将e2e 用例更改为固定期望数值了。

weizhoublue · 2023-10-07T04:23:14Z

pr 描述中， “如期望发送 15 个请求，10 个并发协程执行，当完成 10 个请求后，剩余 5 个请求被 5 个协程分配，但剩下的 5 个协程没有被分配到依然会去请求，影响期望结果”
而之前e2e 偏差容忍是 10%，那 PR 描述不准确，还是 E2E 有 bug 没测试出来

之前的e2e 设置的 qps 以及并发量正好偏差在 10%以内所以用例通过了，现在已经将e2e 用例更改为固定期望数值了。

如果是用绝对值比较，是否可能会出现一些边际效应，例如超时时间的偏差，导致实际多一个或者少一个请求，导致用例时过时不过

ii2day · 2023-10-09T02:06:26Z

pr 描述中， “如期望发送 15 个请求，10 个并发协程执行，当完成 10 个请求后，剩余 5 个请求被 5 个协程分配，但剩下的 5 个协程没有被分配到依然会去请求，影响期望结果”
而之前e2e 偏差容忍是 10%，那 PR 描述不准确，还是 E2E 有 bug 没测试出来

之前的e2e 设置的 qps 以及并发量正好偏差在 10%以内所以用例通过了，现在已经将e2e 用例更改为固定期望数值了。

如果是用绝对值比较，是否可能会出现一些边际效应，例如超时时间的偏差，导致实际多一个或者少一个请求，导致用例时过时不过

client 是根据 qps 来进行发送请求，超时时间内会发送完所有请求，所有请求返回完毕才会结束，若因为超时原因失败，那是否可以考虑是 server 不满足我的任务需求，请求是协程在做，不会阻塞，除非超时时间设置的不合理，才会导致少发请求，但我们在 webhook 中对超时时间进行了检查，不会出现这种情况。

weizhoublue · 2023-10-11T04:14:49Z

client 是根据 qps 来进行发送请求，超时时间内会发送完所有请求，所有请求返回完毕才会结束
----- 如果最后一个请求刚好卡在超时的 0.001 秒错过了呢？

weizhoublue · 2023-10-12T12:47:54Z

本pr是否需要改进

ii2day · 2023-10-12T16:00:10Z

本pr是否需要改进

需要改进，等复用daemonset 的需求搞完，弄此pr

ii2day · 2023-10-17T09:06:56Z

本pr是否需要改进

已改进完成，通过根据 qps 设置定时器发送 channel 控制请求发送，拿到 token 的并发协程才去请求，未拿到 token 的协程继续等待，用例请求误差容忍改为5%。

weizhoublue · 2023-10-17T09:45:26Z

pkg/loadRequest/loadDns/dns_requester.go

@@ -64,6 +67,7 @@ type Work struct {
 	initOnce  sync.Once
 	results   chan *result
 	stopCh    chan struct{}
+	runCh     chan struct{}


-> qosTokenBucket

weizhoublue · 2023-10-17T09:47:11Z

pkg/loadRequest/loadDns/dns_requester.go

+				b.Stop()
+				return
+			case <-ticker.C:
+				b.runCh <- struct{}{}


投放的 debug 日志

weizhoublue · 2023-10-18T04:02:08Z

pkg/loadRequest/loadDns/dns_requester.go

+				b.Stop()
+				return
+			case <-ticker.C:
+				b.Logger.Sugar().Debugf("request token channel len: %d", len(b.qosTokenBucket))


需要 len > 0 有错误日志

weizhoublue · 2023-10-18T04:02:30Z

pkg/loadRequest/loadDns/dns_requester.go

+			select {
+			case <-c:
+				// Reach request duration stop request
+				b.Logger.Sugar().Debugf("request finish remaining number of tokens len: %d", len(b.qosTokenBucket))


需要 len > 0 有错误日志，且任务失败

Signed-off-by: ii2day <ji.li@daocloud.io>

ii2day added cherrypick-release-v0.2 release/feature-changed labels Sep 22, 2023

ii2day requested a review from weizhoublue as a code owner September 22, 2023 08:44

ii2day force-pushed the pr/ii2day/concurrency branch 2 times, most recently from a1b1708 to a7768a9 Compare September 25, 2023 01:47

ii2day force-pushed the pr/ii2day/concurrency branch from a7768a9 to d65736d Compare September 28, 2023 06:42

ii2day force-pushed the pr/ii2day/concurrency branch 3 times, most recently from 8d0370d to e14ccf8 Compare October 17, 2023 08:32

weizhoublue reviewed Oct 17, 2023

View reviewed changes

ii2day force-pushed the pr/ii2day/concurrency branch 6 times, most recently from 70bea9f to 174a717 Compare October 17, 2023 11:46

weizhoublue reviewed Oct 18, 2023

View reviewed changes

ii2day force-pushed the pr/ii2day/concurrency branch 5 times, most recently from aa0488a to 868c9d6 Compare October 19, 2023 03:37

fix conflicts between concurrency and request counts

1139931

Signed-off-by: ii2day <ji.li@daocloud.io>

ii2day force-pushed the pr/ii2day/concurrency branch from 868c9d6 to 1139931 Compare October 19, 2023 06:55

weizhoublue added release/bug and removed release/feature-changed labels Oct 19, 2023

weizhoublue changed the title ~~fix conflicts between concurrency and request counts~~ fix: qps is not correct Oct 19, 2023

weizhoublue merged commit 1aaf685 into main Oct 19, 2023
23 of 24 checks passed

weizhoublue deleted the pr/ii2day/concurrency branch October 19, 2023 13:02

weizhoublue mentioned this pull request Oct 19, 2023

failed to auto cherry pick PR 221 to branch release-v0.2 #252

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: qps is not correct #221

fix: qps is not correct #221

ii2day commented Sep 22, 2023 •

edited

Loading

codecov bot commented Sep 22, 2023 •

edited

Loading

weizhoublue commented Sep 24, 2023

ii2day commented Sep 25, 2023

weizhoublue commented Sep 26, 2023

ii2day commented Sep 27, 2023

weizhoublue commented Oct 7, 2023

ii2day commented Oct 9, 2023

weizhoublue commented Oct 11, 2023

weizhoublue commented Oct 12, 2023

ii2day commented Oct 12, 2023

ii2day commented Oct 17, 2023 •

edited

Loading

weizhoublue Oct 17, 2023

ii2day Oct 19, 2023

weizhoublue Oct 17, 2023

ii2day Oct 19, 2023

weizhoublue Oct 18, 2023

ii2day Oct 19, 2023

weizhoublue Oct 18, 2023 •

edited

Loading

ii2day Oct 19, 2023

fix: qps is not correct #221

fix: qps is not correct #221

Conversation

ii2day commented Sep 22, 2023 • edited Loading

codecov bot commented Sep 22, 2023 • edited Loading

Codecov Report

weizhoublue commented Sep 24, 2023

ii2day commented Sep 25, 2023

weizhoublue commented Sep 26, 2023

ii2day commented Sep 27, 2023

weizhoublue commented Oct 7, 2023

ii2day commented Oct 9, 2023

weizhoublue commented Oct 11, 2023

weizhoublue commented Oct 12, 2023

ii2day commented Oct 12, 2023

ii2day commented Oct 17, 2023 • edited Loading

weizhoublue Oct 17, 2023

Choose a reason for hiding this comment

ii2day Oct 19, 2023

Choose a reason for hiding this comment

weizhoublue Oct 17, 2023

Choose a reason for hiding this comment

ii2day Oct 19, 2023

Choose a reason for hiding this comment

weizhoublue Oct 18, 2023

Choose a reason for hiding this comment

ii2day Oct 19, 2023

Choose a reason for hiding this comment

weizhoublue Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

ii2day Oct 19, 2023

Choose a reason for hiding this comment

ii2day commented Sep 22, 2023 •

edited

Loading

codecov bot commented Sep 22, 2023 •

edited

Loading

ii2day commented Oct 17, 2023 •

edited

Loading

weizhoublue Oct 18, 2023 •

edited

Loading