fix entry overwrite and reduce cache get set

Author: tzongw

redis/connection.py

@@ -1475,7 +1475,7 @@ def send_command(self, *args, **kwargs):
                     with self._pool_lock:
                         while cache_entry.connection_ref.can_read():
                             cache_entry.connection_ref.read_response(push_request=True)
-                    # Check if entry still exists.
+                    # Check if entry still exists, if so it must be cache_entry we got because of cache_lock
                     if self._cache.get(cache_key) is not None:
                         self._current_command_cache_entry = cache_entry
                         return
@@ -1533,17 +1533,12 @@ def read_response(
             cache_key = self._current_command_cache_entry.cache_key
             if response is None:
                 self._cache.delete_by_cache_keys([cache_key])
+                self._current_command_cache_entry = None
                 return response
 
-            cache_entry = self._cache.get(cache_key)
-
-            # Cache only responses that still valid
-            # and wasn't invalidated by another connection in meantime.
-            if cache_entry is not None:
-                cache_entry.status = CacheEntryStatus.VALID
-                cache_entry.cache_value = response
-                self._cache.set(cache_entry)
-
+            # No bother entry exists in cache or not, this ensures we don't overwrite another entry.
+            self._current_command_cache_entry.status = CacheEntryStatus.VALID
+            self._current_command_cache_entry.cache_value = response
             self._current_command_cache_entry = None
 
         return response

tests/test_connection.py

@@ -483,14 +483,6 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection):
         mock_cache.is_cachable.return_value = True
         mock_cache.get.side_effect = [
             None,
-            CacheEntry(
-                cache_key=CacheKey(
-                    command="GET", redis_keys=("foo",), redis_args=("GET", "foo")
-                ),
-                cache_value=CacheProxyConnection.DUMMY_CACHE_VALUE,
-                status=CacheEntryStatus.IN_PROGRESS,
-                connection_ref=mock_connection,
-            ),
             CacheEntry(
                 cache_key=CacheKey(
                     command="GET", redis_keys=("foo",), redis_args=("GET", "foo")
@@ -526,22 +518,14 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection):
         proxy_connection.send_command(*["GET", "foo"], **{"keys": ["foo"]})
         assert proxy_connection.read_response() == b"bar"
         assert proxy_connection._current_command_cache_entry is None
+
+        # cached reply
+        proxy_connection.send_command(*["GET", "foo"], **{"keys": ["foo"]})
         assert proxy_connection.read_response() == b"bar"
+        assert proxy_connection._current_command_cache_entry is None
 
         mock_cache.set.assert_has_calls(
             [
-                call(
-                    CacheEntry(
-                        cache_key=CacheKey(
-                            command="GET",
-                            redis_keys=("foo",),
-                            redis_args=("GET", "foo"),
-                        ),
-                        cache_value=CacheProxyConnection.DUMMY_CACHE_VALUE,
-                        status=CacheEntryStatus.IN_PROGRESS,
-                        connection_ref=mock_connection,
-                    )
-                ),
                 call(
                     CacheEntry(
                         cache_key=CacheKey(