A huge performance regression has been noted in our projects between v8.2 and v8.3 of the client. It seems to be due to the usage of atomics when metrics were added to the client.
Activity
5
icellan10mo agoThe degradation is indeed with metrics turned on, but I think the bigger problem is the overuse of atomic variables in this code path. Without having looked at the code in detail, the metrics bool should not need to be an atomic. At 1 million ops per second, using an atomic tends to become problematic in Go.
mirzakaracic10mo agobigger problem is the overuse of atomic variables in this code
This is expected, since we added additional metrics in the hot path where the new metrics are sourced.
That said, the optimizations that come to mind would impact dynamic configuration—specifically how quickly metricsEnabled propagates. In scenarios where metricsEnabled is not toggled frequently, this isn’t an issue. However, in environments where it is toggled often, we could see misses/false positives.
Your concerns are valid. This will require more thought and input from others before we decide on the best approach forward. I’ll keep you posted, @icellan.
mirzakaracic10mo ago@icellan Have tried running number of loads using client.BatchOperate(...). Was not able to get similar pattern that showed DetailedResultCodeCounts as the bottleneck aka node_stats.updateOrInsert.
What kind of operations have you been performing where you have observed the above pattern?
icellan9mo ago@mirzakaracic We are running millions of ops per second in batches. So, this is a batch update.
Thank you for bringing this to our attention, @icellan. In v8.3.0 we expanded the metrics, adding more data points when detailed metrics are enabled. Because of these additions, the increase/performance hit you’re seeing is expected.
That said, have you noticed this performance degradation specifically after calling client.EnableMetrics(...)?