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

Intelligent addition or reduction of workers #193

Merged
merged 4 commits into from
Oct 11, 2023
Merged

Intelligent addition or reduction of workers #193

merged 4 commits into from
Oct 11, 2023

Conversation

MyNextWeekend
Copy link
Contributor

负载变化时导致并发突增 #173

@MyNextWeekend
Copy link
Contributor Author

because use of context, the dependent version needs to be upgraded.
Please help me check whether this plan can be accepted.

log.Println("The total number of clients required is ", spawnCount)

var gapCount int
if spawnCount > int(r.numClients) {
Copy link
Owner

Choose a reason for hiding this comment

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

We should consider the weight of tasks.

Copy link
Contributor Author

@MyNextWeekend MyNextWeekend Sep 18, 2023

Choose a reason for hiding this comment

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

请见谅,怕表达不清晰,采用中文
我是这样理解的
1、spawnWorkers应该专注于活跃线程的增减

func (r *runner) spawnWorkers(spawnCount int, quit chan bool, spawnCompleteFunc func()) {

2、getTask应该专注于给活跃线程提供任务(按权重提供)
func (r *runner) getTask() *Task {

此处没有理解关于任务权重的想法,还请明确一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在setTask的时候根据权重生成一个runTask []*Task
每个活跃的goroutine将一直getTask 从runTask中获取任务并执行任务
关于权重这块提交了代码,请查阅

Copy link
Owner

Choose a reason for hiding this comment

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

嗯,你是对的,直接用 getTask 来管权重就好。

log.Println("The total number of clients required is ", spawnCount)

var gapCount int
if spawnCount > int(r.numClients) {
Copy link
Owner

Choose a reason for hiding this comment

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

嗯,你是对的,直接用 getTask 来管权重就好。

@@ -30,6 +30,7 @@ type runner struct {

tasks []*Task
totalTaskWeight int
runTask []*Task // goroutine execute tasks according to the list
Copy link
Owner

Choose a reason for hiding this comment

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

为啥要维护这个数组?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

维护这个数组里面存放的需要循环执行的任务(按照权重排序。例如:A-2 B-3 C-4,排序后ABCABCBCC),每个活跃的线程都按照这个顺序执行任务。个人理解可能比随机要好点。
能保证任务执行的权重,也能保证任务都能执行。

runner.go Outdated
stopChan chan bool
// Cancellation method for all running workers(goroutines)
cancelFuncs []context.CancelFunc
mu sync.Mutex
Copy link
Owner

Choose a reason for hiding this comment

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

避免用 mutex,维护的心智负担有点大。有状态机来保证执行顺序了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里看起来可以直接删除锁机制
理由:增减线程命令来自于监听chan得到,本身是非并发的。整个操作不引入协程的话,就可以直接删除锁机制了
但是这样是否合理,大佬可以一起帮忙看一下,我现在提交

Copy link
Owner

Choose a reason for hiding this comment

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

我只是不想在这里用锁,具体的实现有没有问题,要看测试用例。

runner.go Outdated
// stop previous goroutines without blocking
// those goroutines will exit when r.safeRun returns
close(r.stopChan)
go r.spawnWorkers(0, nil)
Copy link
Owner

Choose a reason for hiding this comment

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

靠 spawn 0 来实现 stop,有点拗口。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果把 spawnWorkers方法名修改为 setwWorkerNumber会不会好点

Copy link
Owner

Choose a reason for hiding this comment

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

直接用 stop 就好了吧,那怕是在这里遍历所有 context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,这里可以直接调用reduceWorkers 把当前活跃线程总数丢进去
我修改一下

@myzhan
Copy link
Owner

myzhan commented Sep 19, 2023

这个 PR,需要补充对应的单元测试用例。

@myzhan
Copy link
Owner

myzhan commented Sep 19, 2023

我挺想把这块改平滑的,之前比较粗暴。但我之前的想法,是从 locust 那边改起,还没时间弄。

@MyNextWeekend
Copy link
Contributor Author

image 测试用例已通过

@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2023

Codecov Report

Merging #193 (1fcecb1) into master (67d96b2) will decrease coverage by 0.59%.
The diff coverage is 84.84%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   81.19%   80.60%   -0.59%     
==========================================
  Files          10       10              
  Lines        1521     1542      +21     
==========================================
+ Hits         1235     1243       +8     
- Misses        238      248      +10     
- Partials       48       51       +3     
Flag Coverage Δ
unittests 80.60% <84.84%> (-0.59%) ⬇️

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

Files Coverage Δ
runner.go 82.60% <84.84%> (-0.60%) ⬇️

... and 2 files with indirect coverage changes

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

@myzhan myzhan closed this Oct 11, 2023
@myzhan myzhan reopened this Oct 11, 2023
@myzhan myzhan merged commit f08c0e5 into myzhan:master Oct 11, 2023
3 of 5 checks passed
@myzhan
Copy link
Owner

myzhan commented Oct 11, 2023

已合并,多谢

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.

3 participants