Only shutdown the Redis pool if it is owned by the SDK (#158)

* Only shutdown a Redis pool created by SDK

* Make pool shutdown behavior an option

Author: jacobthemyth

lib/ldclient-rb/impl/integrations/redis_impl.rb

@@ -33,6 +33,8 @@ def initialize(opts)
             @pool = opts[:pool] || ConnectionPool.new(size: max_connections) do
               ::Redis.new(@redis_opts)
             end
+            # shutdown pool on close unless the client passed a custom pool and specified not to shutdown
+            @pool_shutdown_on_close = (!opts[:pool] || opts.fetch(:pool_shutdown_on_close, true))
             @prefix = opts[:prefix] || LaunchDarkly::Integrations::Redis::default_prefix
             @logger = opts[:logger] || Config.default_logger
             @test_hook = opts[:test_hook]  # used for unit tests, deliberately undocumented
@@ -118,6 +120,7 @@ def initialized_internal?
 
           def stop
             if @stopped.make_true
+              return unless @pool_shutdown_on_close
               @pool.shutdown { |redis| redis.close }
             end
           end

lib/ldclient-rb/integrations/redis.rb

@@ -45,6 +45,7 @@ def self.default_prefix
       # @option opts [Integer] :expiration (15)  expiration time for the in-memory cache, in seconds; 0 for no local caching
       # @option opts [Integer] :capacity (1000)  maximum number of items in the cache
       # @option opts [Object] :pool  custom connection pool, if desired
+      # @option opts [Boolean] :pool_shutdown_on_close whether calling `close` should shutdown the custom connection pool.
       # @return [LaunchDarkly::Interfaces::FeatureStore]  a feature store object
       #
       def self.new_feature_store(opts)

lib/ldclient-rb/redis_store.rb

@@ -35,6 +35,7 @@ class RedisFeatureStore
     # @option opts [Integer] :expiration  expiration time for the in-memory cache, in seconds; 0 for no local caching
     # @option opts [Integer] :capacity  maximum number of feature flags (or related objects) to cache locally
     # @option opts [Object] :pool  custom connection pool, if desired
+    # @option opts [Boolean] :pool_shutdown_on_close whether calling `close` should shutdown the custom connection pool.
     #
     def initialize(opts = {})
       core = LaunchDarkly::Impl::Integrations::Redis::RedisFeatureStoreCore.new(opts)

spec/redis_feature_store_spec.rb

@@ -1,3 +1,4 @@
+require "connection_pool"
 require "feature_store_spec_base"
 require "json"
 require "redis"
@@ -27,11 +28,11 @@ def clear_all_data
 
 describe LaunchDarkly::RedisFeatureStore do
   subject { LaunchDarkly::RedisFeatureStore }
-  
+
   break if ENV['LD_SKIP_DATABASE_TESTS'] == '1'
 
   # These tests will all fail if there isn't a Redis instance running on the default port.
-  
+
   context "real Redis with local cache" do
     include_examples "feature_store", method(:create_redis_store), method(:clear_all_data)
   end
@@ -59,7 +60,7 @@ def make_concurrent_modifier_test_hook(other_client, flag, start_version, end_ve
     flag = { key: "foo", version: 1 }
     test_hook = make_concurrent_modifier_test_hook(other_client, flag, 2, 4)
     store = create_redis_store({ test_hook: test_hook })
-    
+
     begin
       store.init(LaunchDarkly::FEATURES => { flag[:key] => flag })
 
@@ -77,7 +78,7 @@ def make_concurrent_modifier_test_hook(other_client, flag, start_version, end_ve
     flag = { key: "foo", version: 1 }
     test_hook = make_concurrent_modifier_test_hook(other_client, flag, 3, 3)
     store = create_redis_store({ test_hook: test_hook })
-    
+
     begin
       store.init(LaunchDarkly::FEATURES => { flag[:key] => flag })
 
@@ -89,4 +90,32 @@ def make_concurrent_modifier_test_hook(other_client, flag, start_version, end_ve
       other_client.close
     end
   end
+
+  it "shuts down a custom Redis pool by default" do
+    unowned_pool = ConnectionPool.new(size: 1, timeout: 1) { Redis.new({ url: "redis://localhost:6379" }) }
+    store = create_redis_store({ pool: unowned_pool })
+
+    begin
+      store.init(LaunchDarkly::FEATURES => { })
+      store.stop
+
+      expect { unowned_pool.with {} }.to raise_error(ConnectionPool::PoolShuttingDownError)
+    ensure
+      unowned_pool.shutdown { |conn| conn.close }
+    end
+  end
+
+  it "doesn't shut down a custom Redis pool if pool_shutdown_on_close = false" do
+    unowned_pool = ConnectionPool.new(size: 1, timeout: 1) { Redis.new({ url: "redis://localhost:6379" }) }
+    store = create_redis_store({ pool: unowned_pool, pool_shutdown_on_close: false })
+
+    begin
+      store.init(LaunchDarkly::FEATURES => { })
+      store.stop
+
+      expect { unowned_pool.with {} }.not_to raise_error(ConnectionPool::PoolShuttingDownError)
+    ensure
+      unowned_pool.shutdown { |conn| conn.close }
+    end
+  end
 end