Thread contention in TimeWindowQuantiles because of `synchronized`

[t3hnar]

Hello guys,
Use of Summary gives considerable performance penalty because of multiple synchronized calls in .observe flow.
Will you able to fix this?

[brian-brazil]

This is a fundamental issue with client-side quantiles. If you remove the quantiles or use a Histogram instead, you can avoid this specific problem.

[t3hnar]

This is a fundamental issue with client-side quantiles

do you mean that this is not solvable without synchronized problem in computer science ?

[brian-brazil]

I've yet to see a solution that doesn't require this.

I'll investigate if we could use atomics / compare-and-swap instead of locks / synchronized, as that might yield better performance for our use cases.

I might have found something strange, but I could be misunderstanding the details. 🤔

It looks like TimeWindowQuantiles#get reads current bucket, but TimeWindowQuantiles#insert writes all buckets.
Note that CKMSQuantiles#insert is synchronized, so the writes are causing unnecessary thread contention.
I'm wondering what's the point of writing buckets which are never read?

Edit: I was too quick to post this comment, other buckets could be read in the future, when they'll become the current bucket.

[brian-brazil]

They're rotated through. Having a quantile that represents recent data (rather than since process start) is tricky.

I’ll check out HdrHistogram as the next experiment, as it looks like it’s designed exactly for our use case, but I’m not sure how well it integrates with Prometheus.

We'll continue with profiling the alternatives and experiments (#481, #482, #483, #484) under realistic workload(s), as I can't produce similar thread contention with synthetic benchmarks.

I'm still working on testing and profiling #482 + #484 using different use cases.
I won't be able to test them in isolation as I was asked to test them together.
I won't be able to test #481 and #483 as those are mutually exclusive to #484.

I'm sorry, but I still couldn't get back to this, but I'll try this week. 🙁

[brian-brazil]

#481 is in now.