-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Currently, there is no documented requirement for custom exemplar reservoirs to be concurrent-safe. We should consider adding such a requirement, although there is a good chance that will break existing users.
This is an oversight that will likely prevent us from making exemplar reservoirs performant. The reason that the default reservoir capacity is the number of CPUs is to reduce lock contention when updating individual elements. But currently we hold a global per-instrument lock while accessing the reservoir, which prevents any concurrent access to the reservoir at all.
This isn't surfaced in our benchmarks for sync measure today, as we don't benchmark with sampled trace contexts. We should add a benchmark like this before making any changes described above.
As a strawman, I would propose a feature flag to move synchronization of the exemplar reservoir from outside the reservoir to inside the reservoir to help users adapt to the change.