Merge pull request rails#54460 from ryush00/cache-pool-timeout-and-tests

Handle ConnectionPool::TimeoutError in Redis and Memcached stores

Author: byroot

activesupport/lib/active_support/cache/mem_cache_store.rb

@@ -273,7 +273,7 @@ def deserialize_entry(payload, raw: false, **)
 
         def rescue_error_with(fallback)
           yield
-        rescue Dalli::DalliError, ConnectionPool::Error => error
+        rescue Dalli::DalliError, ConnectionPool::Error, ConnectionPool::TimeoutError => error
           logger.error("DalliError (#{error}): #{error.message}") if logger
           ActiveSupport.error_reporter&.report(
             error,

activesupport/lib/active_support/cache/redis_cache_store.rb

@@ -497,7 +497,7 @@ def supports_expire_nx?
 
         def failsafe(method, returning: nil)
           yield
-        rescue ::Redis::BaseError, ConnectionPool::Error => error
+        rescue ::Redis::BaseError, ConnectionPool::Error, ConnectionPool::TimeoutError => error
           @error_handler&.call(method: method, exception: error, returning: returning)
           returning
         end

activesupport/test/cache/behaviors/connection_pool_behavior.rb

@@ -7,10 +7,10 @@ def test_connection_pool
     threads = []
 
     emulating_latency do
-      cache = ActiveSupport::Cache.lookup_store(*store, { pool: { size: 2, timeout: 1 } }.merge(store_options))
+      cache = ActiveSupport::Cache.lookup_store(*store, { pool: { size: 2, timeout: 0.1 } }.merge(store_options))
       cache.read("foo")
 
-      assert_raises Timeout::Error do
+      assert_nothing_raised do
         # One of the three threads will fail in 1 second because our pool size
         # is only two.
         3.times do
@@ -28,6 +28,36 @@ def test_connection_pool
     Thread.report_on_exception = original_report_on_exception
   end
 
+  def test_connection_pool_fetch
+    Thread.report_on_exception, original_report_on_exception = false, Thread.report_on_exception
+
+    threads = []
+    results = []
+
+    emulating_latency do
+      cache = ActiveSupport::Cache.lookup_store(*store, { pool: { size: 2, timeout: 0.1 } }.merge(store_options))
+      value = SecureRandom.alphanumeric
+      base_key = "latency:#{SecureRandom.uuid}"
+
+      assert_nothing_raised do
+        # One of the three threads will fail in 1 second because our pool size
+        # is only two.
+        3.times do |i|
+          threads << Thread.new do
+            cache.fetch("#{base_key}:#{i}") { value }
+          end
+        end
+
+        results = threads.map(&:value)
+        assert_equal [value] * 3, results, "All threads should return the same value"
+      end
+    ensure
+      threads.each(&:kill)
+    end
+  ensure
+    Thread.report_on_exception = original_report_on_exception
+  end
+
   def test_no_connection_pool
     threads = []
 

activesupport/test/cache/stores/redis_cache_store_test.rb

@@ -10,7 +10,7 @@
 class SlowRedis < Redis
   def get(key)
     if /latency/.match?(key)
-      sleep 3
+      sleep 0.2
       super
     else
       super