Fixes average stats bug #436
Conversation
| | ||
| for stats in SERVER_STATS.read().values() { | ||
| stats.address_stats().update_averages(); | ||
| // Hold read lock for duration of update to retain all server stats |
There was a problem hiding this comment.
I am confused here. We only have one instance of the collector running, so there is only one task updating averages. How can we run update_averages() twice or more even with multiple server connections connected to the same address?
There was a problem hiding this comment.
This issue here comes from all the servers having the same reference to the AddressStats. When we update address stat's old_total. The next server that we try to update averages for will see no difference between the current total and the old total cause it was updated in the previous iteration of the loop
There was a problem hiding this comment.
I have an example of this in the description of the PR
There was a problem hiding this comment.
Aaah okay. Confusing! Arcs are tricky. Ok, sounds good, I think this is fine for now. Would be good to investigate further and maybe remove the leaked reference?
When there are multiple concurrent active servers, the stats reporting reports a 0 value average. This is because of the old value being updated and then the average being overridden when a new server pointing to the same address stats gets updated.
This PR fixes this by running a check to make sure that we only ever do this once for any given AddressStats Arc object.