Conversation
anikielx
force-pushed
the
test_io_class_stats_core_cache
branch
from
72221a9 to
c4f3246
Compare
|
retest this please |
| with TestRun.step("Start caches (one for each supported cache mode) and add core devices."): | ||
| caches = [casadm.start_cache(dev, cache_mode=cache_mode, force=True) | ||
| for dev, cache_mode in zip(cache_devices, CacheMode)] |
There was a problem hiding this comment.
CAS support 5 cache modes: WO, WB, WT, WA and PT. You should assign len(CacheMode) to number_of_caches in line 24 instead of assigning the raw number
| s = '' if per_core else '(s)' | ||
| stats_pt_wa = [f'writes to exported object{s}', f'total to/from exported object{s}', | ||
| f'writes to core{s}', f'total to/from core{s}'] | ||
| stats_wb = ['occupancy', 'dirty', 'write full misses', 'write total', | ||
| f'writes to exported object{s}', f'total to/from exported object{s}', | ||
| 'writes to cache', 'total to/from cache'] | ||
| stats_wt = ['occupancy', 'clean', 'write full misses', 'write total', | ||
| f'writes to exported object{s}', f'total to/from exported object{s}', | ||
| 'writes to cache', 'total to/from cache', | ||
| f'writes to core{s}', f'total to/from core{s}'] |
There was a problem hiding this comment.
It can be defined outside of the loop
There was a problem hiding this comment.
Instead of associating lists of stats relevant for the particular cache modes you should use trait-relevant stats association. That way the test will become more flexible for any modifications like adding a new cache mode or adding a new statistic.
You can grep older tests in search of CacheModeTrait to see the usage of trait API.
InsertWrite means a cache mode inserts data into cache when writing
InsertRead means a cache mode inserts data into cache when reading
LazyWrites means a cache mode inserts data into cache when writing but it doesn't flush on core device
| if cache_mode == CacheMode.PT or cache_mode == CacheMode.WA: | ||
| expected_value = size_in_blocks if name in stats_pt_wa else 0 | ||
| check_value(name, value, expected_value) | ||
|
|
||
| elif cache_mode == CacheMode.WB: | ||
| expected_value = size_in_blocks if name in stats_wb else 0 | ||
| check_value(name, value, expected_value) | ||
|
|
||
| elif cache_mode == CacheMode.WT: | ||
| expected_value = size_in_blocks if name in stats_wt else 0 | ||
| check_value(name, value, expected_value) |
There was a problem hiding this comment.
Missing case for WO. If you apply my suggestion with using traits you won't need to add a new extra case
| if cache_mode == CacheMode.PT: | ||
| expected_value = 0 | ||
| epsilon_percentage = 0 | ||
| check_perc_value(name, value, expected_value, epsilon_percentage) | ||
|
|
||
| elif cache_mode == CacheMode.WA: | ||
| expected_value = 0 | ||
| epsilon_percentage = 0.5 if name == 'occupancy' else 0 | ||
| check_perc_value(name, value, expected_value, epsilon_percentage) | ||
|
|
||
| elif cache_mode == CacheMode.WB: | ||
| occupancy = 100 * size_in_blocks / cache.size.get_value() | ||
| expected_value = 100 if name == 'dirty' else \ | ||
| occupancy if name == 'occupancy' else 0 | ||
| epsilon_percentage = 0.5 if name in ('dirty', 'occupancy') else 0 | ||
| check_perc_value(name, value, expected_value, epsilon_percentage) | ||
|
|
||
| elif cache_mode == CacheMode.WT: | ||
| occupancy = 100 * size_in_blocks / cache.size.get_value() | ||
| expected_value = 100 if name == 'clean' else \ | ||
| occupancy if name == 'occupancy' else 0 | ||
| epsilon_percentage = 0.5 if name in ('clean', 'occupancy') else 0 | ||
| check_perc_value(name, value, expected_value, epsilon_percentage) |
There was a problem hiding this comment.
Missing case for WO. If you apply my suggestion with using traits you won't need to add a new extra case
test/functional/tests/io_class/test_io_class_stats_core_cache.py
Outdated
Show resolved
Hide resolved
|
|
||
| core.reset_counters() | ||
| cache.purge_cache() | ||
| drop_caches() |
There was a problem hiding this comment.
In case a cache device has some dirty data purge will trigger flushing which will lead to nonzero values in stats thus suggest resetting stats after purge and drop caches
There was a problem hiding this comment.
Btw instead of resetting stats of each core separately, how about using cache.reset_counters()? Additionally, I would move it to a separate step with a separate cache-based loop
|
I would say that testing more than one cache in a single iteration overcomplicates the code whereas the benefits of this approach are negligible. It could be a good idea to simplify it by testing only WT cache mode and leaving only one cache. In that case the most of my comments become irrelevant |
anikielx
force-pushed
the
test_io_class_stats_core_cache
branch
from
c4f3246 to
79c571f
Compare
Signed-off-by: Adriana Nikelska <adrianax.nikielska@intel.com>
|
needs modifications: the most important are parametrize by CacheMode and check stats logic |
4 participants
Signed-off-by: Adriana Nikelska adrianax.nikielska@intel.com