lukevenediger / statsd.net

A high-performance stats collection service that feeds into Graphite and aimed at small- to large-scale windows environments.
MIT License
122 stars 25 forks source link

Race conditions when updating guage value #29

Open mariuszwojcik opened 10 years ago

mariuszwojcik commented 10 years ago

Hi Luke,

I was looking at Gauge implementation as we want to use statsd.net in our applications, and noticed that there is no guarantee of processing messages in order. When running LogMultipleValuesFromOneGauge_OneGraphiteLine_Success test I noticed that it fails most of the time. Further code analysis shows that gauge updates are processed asynchronously on multiple threads allowing for gauge being set to other value than last update.

The easy solution will be to set MaxDegreeOfParallelism on gauge's ActionBlock to 1, but that will have an impact on general performance. Any thoughts on better approach?

Regards,

Mariusz

lukevenediger commented 10 years ago

Hey Mariusz,

I agree with you - there is a race condition here. The trouble comes in with the ConcurrentDictionary, since all updates to the dictionary become tasks themselves and will be executed some time in the future. I'm trying an approach where I keep a counter for each message that's received, then keep every message, and lastly work out the result when the timer pulses.

Cheers, Luke

mariuszwojcik commented 10 years ago

Hi Luke,

I see there are two ways to solve the problem. First is to set MaxDegreeOfParallelism to 1 and keep code quick and simple. Other solution is as you mentioned versioning messages, storing then and then determining what the result should be on timer pulse. I was wondering whether it is beneficial to keep higher degree of parallelism by increased cost of code execution and memory utilisation. Especially when the current code is so simple. I do not have much experience with TPL dataflow, but my experience with concurrent, distributed systems was that sometimes running simple code without concurrency gives better performance than more complicated multithreaded solutions.

I am just speculating here. Currently in my project I am using Gauges with MaxDegreeOfParallelism set to 1. We have couple of dozen gauges which are flushed to Graphite every 10 secs and we are quite happy with it.

Cheers, Mariusz