Create Sysbench tool and testcases using it for CPU/Memory/FileIO performance #3075
Conversation
smit-gardhariya
requested review from
squirrelsc and
LiliDeng
as code owners
smit-gardhariya
force-pushed
the
sgardhariya/sysbench_tool
branch
3 times, most recently
from
4f150a9 to
225ce70
Compare
smit-gardhariya
force-pushed
the
sgardhariya/sysbench_tool
branch
4 times, most recently
from
7caff91 to
7bf8623
Compare
smit-gardhariya
force-pushed
the
sgardhariya/sysbench_tool
branch
10 times, most recently
from
e7a3d69 to
30a9480
Compare
smit-gardhariya
force-pushed
the
sgardhariya/sysbench_tool
branch
2 times, most recently
from
0130a17 to
739a5e9
Compare
lisa/messages.py
Outdated
| @@ -125,6 +125,22 @@ class PerfMessage(MessageBase): | |||
| role: str = "" | |||
| test_result_id: str = "" | |||
|
|
|||
| # Sysbench common params | |||
There was a problem hiding this comment.
The PerfMessage is the base class for all perf messages. Please either move them into existing perf message, or new perf messages. The fields shouldn't be tool oriented, they should be metrics oriented.
There was a problem hiding this comment.
These are the common one for sysbench. I put them here as i dont want to have redundant params for all child PerfMessage classes.
I created new perf message dataclass now for sysbench for all those common params and inherited it for Disk/CPU/Memory perf dataclasses
There was a problem hiding this comment.
The perf messages should be metrics oriented, instead of tool oriented. It's easy to create dashboard and explain by metrics concepts. It's the same to sysbench common fields. It looks many common fields are not help a lot as a "common" field, the tested metrics are the key information of a perf message. We need to abstract the values to perf metrics.
For example, it's easy to understand x_latency_ms in network tests, but hard to understand in CPU tests, and I'm not sure what does it mean in the mem tests. The "events" number is highly depends on the test scenarios and settings.
There was a problem hiding this comment.
If you look at the sysbench output it will always give latency metrics like min/max/avg. That was the reason to put it under common sysbench metrics
lisa/tools/sysbench.py
Outdated
| test_name, | ||
| other_fields, | ||
| ) | ||
| print(subtest_msg) |
There was a problem hiding this comment.
remove the debug lines. You can add console notifier to print all messages in debug level.
There was a problem hiding this comment.
It's still here, not be removed.
There was a problem hiding this comment.
Done now. I added it later again and still pushed it.
lisa/messages.py
Outdated
| @@ -168,6 +190,38 @@ class DiskPerformanceMessage(PerfMessage): | |||
| randwrite_iops: Decimal = Decimal(0) | |||
| randwrite_lat_usec: Decimal = Decimal(0) | |||
|
|
|||
| # Sysbench FileIO params | |||
| read_io_per_sec: Decimal = Decimal(0) | |||
There was a problem hiding this comment.
Please try best to reuse above fields, like read_io_per_sec should be read_iops above. So the data from different tools can be compared together.
There was a problem hiding this comment.
Please check all other fields too.
- Make sure it's a common concept, instead of sysbench concept.
- Create it in the Disk message, if it doesn't exist. But not group them like "sysbench params".
There was a problem hiding this comment.
Will check on this
lisa/messages.py
Outdated
|
|
||
| @dataclass | ||
| class MemoryPerformanceMessage(PerfMessage): | ||
| total_operations: Decimal = Decimal(0) |
There was a problem hiding this comment.
operations_per_sec is easier to understand, if it includes the time unit. And similar to other fields.
There was a problem hiding this comment.
There are 2 field for memory perf message. One is total_operation and other is operations_per_second.
There was a problem hiding this comment.
If so, the total_operation doesn't need here. It's hard to compare cross different settings. We don't need to save all details in sysbench output.
There was a problem hiding this comment.
Removed it now
lisa/messages.py
Outdated
| @@ -149,6 +165,12 @@ class PerfMessage(MessageBase): | |||
| ) | |||
|
|
|||
|
|
|||
| @dataclass | |||
| class CPUPerformanceMessage(PerfMessage): | |||
| cpu_max_prime: Decimal = Decimal(0) | |||
There was a problem hiding this comment.
Is it a test parameter to calculate max prime? If so, use a field like "benchmark", and value is "prime". So it's easy to extend to other perf methodologies.
There was a problem hiding this comment.
It is max number till which sysbench will try to get the prime numbers to generate CPU stress. Default is 10000 i.e. sysbench cpu run would try to get prime number upto 10000.
There was a problem hiding this comment.
It's a test approach. If it's set as a column name, it's hard to extend other tests in the same CPU table. So, the columns should be like below.
benchmark: prime_10000
max_cpu_speed: ???
lisa/tools/sysbench.py
Outdated
| threads: int = 1, | ||
| events: int = 0, | ||
| time_limit: int = 10, | ||
| memory_block_size_in_kb: int = 1, |
There was a problem hiding this comment.
Those arguments are not set in test case level too, they should be removed, unless they are used. Similar for fild and cpu.
There was a problem hiding this comment.
That is correct that we are not using them as of now as we are running with default values. There is plan to run the CPU/Memory perf test using sysbench in near future for our validation stack which would required to run sysbech to be run with different parameter.
Those args are necessary to expose all command-line parameter of sysbench.
I would suggest if we can keep this so we dont need or need minimal changes at tool level. Please let me know your thoughts on this
There was a problem hiding this comment.
Let's add them back in "near future", when it's used. That would be clearer than add "all" now.
smit-gardhariya
force-pushed
the
sgardhariya/sysbench_tool
branch
4 times, most recently
from
03671cc to
6c5d058
Compare
| memory_access_mode: List[str] = ["seq", "rnd"] | ||
| sysbench = node.tools[Sysbench] | ||
|
|
||
| for op in memory_operation: |
There was a problem hiding this comment.
similar to fileio, please move the loop into the tool.
lisa/tools/sysbench.py
Outdated
| raise LisaException( | ||
| f"Invalid memory_scope. Valid memory_scope: {valid_mem_scope}" | ||
| ) | ||
| elif memory_oper not in valid_mem_operation: |
There was a problem hiding this comment.
Similar with fileio, use 3 if, instead of elif on unrelated checks.
| @dataclass | ||
| class MemoryPerformanceMessage(PerfMessage, SysbenchPerfMessage): | ||
| total_operations: Decimal = Decimal(0) | ||
| total_mib_transferred: int = 0 |
There was a problem hiding this comment.
Should it be total_mib_transferred_per_sec? It's easy to cross check on different test settings.
There was a problem hiding this comment.
We have 2 fields for this. total_mib_transferred and mib_per_second
Here is the reference output. We are fetching both the figure under above metrics.
64190.27 MiB transferred (6417.03 MiB/sec)
There was a problem hiding this comment.
It looks similar to total operations, it's not a metrics to measure perf. We don't need to collect all information. The comparable metrics is important. Other data can be found in log for troubleshooting.
This commit has changes to add sysbench as tool to lisa This is needed for CPU/Memory/Fileio perf test using sysbench tool. Signed-off-by: Smit Gardhariya <[email protected]>
This commit has changes to add testcases for CPU/Memory/FileIO using sysbench tool of LISA. Signed-off-by: Smit Gardhariya <[email protected]>
smit-gardhariya
force-pushed
the
sgardhariya/sysbench_tool
branch
from
6c5d058 to
b551a1b
Compare
This PR has 2 commits.