graphite-project / carbon

Carbon is one of the components of Graphite, and is responsible for receiving metrics over the network and writing them down to disk using a storage backend.
http://graphite.readthedocs.org/
Apache License 2.0
1.51k stars 490 forks source link

Fix carbon cache locking #829

Closed piotr1212 closed 5 years ago

piotr1212 commented 5 years ago

Add lock when calling choose_item(), this seemed to cause the issues in #822 #815. Remove previously added locks to counts and watermarks as they deadlocked with the new lock. Improve locking in store(), it would lock after checking if the timestamp is in the cache. The cache could have been altered between checking and locking.

codecov-io commented 5 years ago

Codecov Report

Merging #829 into master will decrease coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #829      +/-   ##
==========================================
- Coverage   50.15%   50.14%   -0.02%     
==========================================
  Files          37       37              
  Lines        3443     3442       -1     
  Branches      495      495              
==========================================
- Hits         1727     1726       -1     
  Misses       1604     1604              
  Partials      112      112
Impacted Files Coverage Δ
lib/carbon/cache.py 80.79% <100%> (-0.13%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update fbbc5c1...8e94e48. Read the comment docs.

deniszh commented 5 years ago

I think that should be merged, right, @piotr1212 ?

piotr1212 commented 5 years ago

Would be nice to have some more regression testing.