Merge pull request #27 from luizalabs/ro-circuit_breaker_race_condition-fix

Delete failure cache key when circuit opens

Author: RomuloOliveira

.travis.yml

@@ -3,6 +3,7 @@ python:
   - "2.7"
   - "3.4"
   - "3.5"
+  - "3.6"
 
 cache:
   directories:

django_toolkit/fallbacks/circuit_breaker.py

@@ -36,6 +36,11 @@ def total_failures(self):
     def open_circuit(self):
         self.cache.set(self.circuit_cache_key, True, self.circuit_timeout)
 
+        # Delete the cache key to mitigate multiple sequentials openings
+        # when a key is created accidentally without timeout (from an incr
+        # operation)
+        self.cache.delete(self.failure_cache_key)
+
         logger.critical(
             'Open circuit for {failure_cache_key} {cicuit_cache_key}'.format(
                 failure_cache_key=self.failure_cache_key,
@@ -68,6 +73,8 @@ def __exit__(self, exc_type, exc_value, traceback):
                 raise self.max_failure_exception
 
     def _increase_failure_count(self):
+        # Between the cache.add and cache.incr, the cache MAY expire,
+        # which will lead to a circuit that will eventually open
         self.cache.add(self.failure_cache_key, 0, self.max_failure_timeout)
         total = self.cache.incr(self.failure_cache_key)
 

tests/fallbacks/test_circuit_breaker.py

@@ -20,6 +20,10 @@ def fail_function():
 
 class TestCircuitBreaker:
 
+    @pytest.fixture(autouse=True)
+    def clear_cache(self):
+        cache.clear()
+
     def test_success_result(self):
         with CircuitBreaker(
             cache=cache,
@@ -113,4 +117,23 @@ def test_should_not_increment_fail_when_circuit_is_open(self):
 
                 fail_function()
 
-        assert cache.get(failure_cache_key) == max_failures
+        assert not cache.get(failure_cache_key)
+
+    def test_should_delete_count_key_when_circuit_is_open(self):
+        failure_cache_key = 'circuit'
+
+        cache.set(failure_cache_key, 1)
+
+        with pytest.raises(MyException):
+            with CircuitBreaker(
+                cache=cache,
+                failure_cache_key=failure_cache_key,
+                max_failures=2,
+                max_failure_exception=MyException,
+                catch_exceptions=(ValueError,),
+            ) as circuit_breaker:
+                fail_function()
+
+            assert circuit_breaker.is_circuit_open
+
+        assert cache.get(failure_cache_key) is None