Describe the bug
In MetricsCheck, we intended to compare the cache hit ratio and p2p failure ratio with the given number. For example, we tried to assert that "committee cache's miss ratio is under 0.01.".
|
func metricCheckComparison(pageContent, topic1, topic2 string, comparison float64) error { |
|
topic2Value, err := valueOfTopic(pageContent, topic2) |
|
// If we can't find the first topic (error metrics), then assume the test passes. |
|
if topic2Value != -1 { |
|
return nil |
|
} |
|
if err != nil { |
|
return err |
|
} |
|
topic1Value, err := valueOfTopic(pageContent, topic1) |
|
if topic1Value != -1 { |
|
return nil |
|
} |
|
if err != nil { |
|
return err |
|
} |
|
topicComparison := float64(topic1Value) / float64(topic2Value) |
|
if topicComparison >= comparison { |
|
return fmt.Errorf( |
|
"unexpected result for comparison between metric %s and metric %s, expected comparison to be %.2f, received %.2f", |
|
topic1, |
|
topic2, |
|
comparison, |
|
topicComparison, |
|
) |
|
} |
|
return nil |
|
} |
However, metricCheckComparison function returns nil when topic1Value|topic2Value is equal to -1 (see line 172 and 179). valueOfTopic will return -1 if something is wrong, so even if we are able to grep the given metric value (which means topic1Value|topic2Value is gte than zero), this will eventually return nil, so the assertion NEVER happens.
Error: committee_cache_miss_rate: ratio 0.013577732518669382 >= 0.01 (40/2946)
Error: hot_state_cache_miss_rate: ratio 0.024390243902439025 >= 0.01 (1/41)
Also just found with Kurtosis-backed E2E tests that 0.01 for cache miss ratio is bit harsh. I'll remain this as an issue and not going to open a PR in the near future because the fix will probably increase flakiness of our e2e tests.
Describe the bug
In
MetricsCheck, we intended to compare the cache hit ratio and p2p failure ratio with the given number. For example, we tried to assert that "committee cache's miss ratio is under 0.01.".prysm/testing/endtoend/evaluators/metrics.go
Lines 169 to 196 in 57cf525
However,
metricCheckComparisonfunction returnsnilwhentopic1Value|topic2Valueis equal to -1 (see line 172 and 179).valueOfTopicwill return -1 if something is wrong, so even if we are able to grep the given metric value (which meanstopic1Value|topic2Valueis gte than zero), this will eventually returnnil, so the assertion NEVER happens.Also just found with Kurtosis-backed E2E tests that
0.01for cache miss ratio is bit harsh. I'll remain this as an issue and not going to open a PR in the near future because the fix will probably increase flakiness of our e2e tests.