Skip to content

Conversation

@danhermann
Copy link
Contributor

For monotonically-increasing metrics, the tradeoff of LongAdder's lower contention for non-atomic value reads tends not to cause problems. The "IngestCurrent" metric is more of a flag-type metric that should hover between zero and a small positive number with frequent increment and decrement operations. In that case, LongAdder's non-atomic reads can result in negative values being returned due to out-of-order application of the increment and decrement operations. Those negative values both confuse users and fail during serialization. For that specific metric, AtomicLong's guarantees are necessary.

Fixes #52411, fixes #52406.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Stats)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danhermann danhermann merged commit 57fa346 into elastic:master Feb 24, 2020
@danhermann danhermann deleted the 52411_negative_ingest_stats branch February 24, 2020 13:09
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Feb 25, 2020
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stats based on LongAdder can return unexpected values [CI] IndexStatsIT.testConcurrentIndexingAndStatsRequests failure in 7.6

4 participants