Add CPU benchmark task #2066
Add CPU benchmark task #2066
Conversation
AbdelrahmanElawady
requested review from
muhamadazmy,
delandtj,
MarioBassem,
ashraffouda,
rawdaGastan and
xmonader
as code owners
| ) | ||
|
|
||
| // CPUBenchmarkTask defines CPU benchmark task data. | ||
| type CPUBenchmarkTask struct { |
There was a problem hiding this comment.
please add more explanation on the fields, e.g taskID is that a sequential string?a uuid?or else?
schedule: i'm assuming that's a cron string? let's clear the ambiguity please
| } | ||
|
|
||
| // CPUBenchmarkResult holds CPU benchmark results with the workloads number during the benchmark. | ||
| type CPUBenchmarkResult struct { |
There was a problem hiding this comment.
Is the Single/Multi process or a thread? can we add a few more info and even change the name to a include the full name (or that's because of the json marshalling?)
There was a problem hiding this comment.
I can change the name of the fields but the data returned from zos.perf.get_all will still be single, multi and threads, is that fine?
pkg/perf/cpubench_task.go
Outdated
| func NewCPUBenchmarkTask() CPUBenchmarkTask { | ||
| return CPUBenchmarkTask{ | ||
| taskID: "CPUBenchmark", | ||
| schedule: "0 0 */6 * * *", |
There was a problem hiding this comment.
can we move that cron string to a const and be used here?
pkg/perf/cpubench_task.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("failed to execute cpubench command: %w", err) | ||
| } | ||
| cpubenchData := struct { |
There was a problem hiding this comment.
Why do we need another anonymous struct while we have CPUBenchmarkResult?
There was a problem hiding this comment.
as the JSON output does not have the workloads field and I don't know what the behavior of json package with missing fields.
There was a problem hiding this comment.
Hmm I think you can just do json:"-" for the field
There was a problem hiding this comment.
I can't use that because it will be ignored when querying the task data. I can just remove the anonymous struct and json ,I believe, will set it to the default value: 0
pkg/perf/cpubench_task.go
Outdated
| @@ -9,27 +9,32 @@ import ( | |||
| "github.com/threefoldtech/zos/pkg/stubs" | |||
| ) | |||
|
|
|||
| const cpuBenchmarkTaskID = "CPUBenchmark" | |||
| const cpuBenchmarkCronScheule = "0 0 */6 * * *" | |||
pkg/perf/monitor.go
Outdated
| return context.WithValue(ctx, zbusClient{}, client) | ||
| } | ||
|
|
||
| func getZbusClient(ctx context.Context) zbus.Client { |
There was a problem hiding this comment.
| func getZbusClient(ctx context.Context) zbus.Client { | |
| func GetZbusClient(ctx context.Context) zbus.Client { |
this should be public because external tasks can still use it
Description
Add benchmark task to run 4 times a day and reports the workload numbers during the benchmark.
Issues