search

acquia / statsgod

A Statsd implementation written in GO lang
Apache License 2.0
119 stars 8 forks source link

Concurrency and data corruption #97

Open jjneely opened 8 years ago

jjneely commented 8 years ago

With the concurrency setting in the configuration statsgod will setup multiple consumers for the relayChannel channel:

https://github.com/acquia/statsgod/blob/master/statsgod.go#L94

Each consumer maintains its own hash of metrics. So how does this not cause data corruption when the same metric key is handled by more than one of the running consumers? It looks like the individual flush cycles would end up overwriting the same metric from another consumer's flush cycle.

kevinhankens commented 8 years ago

@jjneely thanks for the report! I believe that you are correct. The relay concurrency is a feature that we didn't end up using, but sadly did not get enough attention before going into master.

So, it's possible that the idea of concurrent relays is just fundamentally not going to work. Do you see any alternative, or do you think that we drop concurrent relays altogether?

jjneely commented 8 years ago

In the branch I'm working on I removed the for loop there. Otherwise, we need a common data structure, locking, and a new thread to handling flushing.

I'd much rather depend on the channel for synchronization there. I haven't looked at how long we spend in various stages of the code so...

kevinhankens commented 8 years ago

That sounds good to me. We will also want to make sure that we remove the configuration around config.Relay.Concurrency.