]> cat aescling's git repositories - mastodon.git/commitdiff
Fix opening and closing Redis connections instead of using a pool (#18171)
authorEugen Rochko <eugen@zeonfederated.com>
Fri, 29 Apr 2022 20:43:07 +0000 (22:43 +0200)
committersingle-right-quote <11325618-aescling@users.noreply.gitlab.com>
Thu, 5 May 2022 17:49:12 +0000 (13:49 -0400)
* Fix opening and closing Redis connections instead of using a pool

* Fix Redis connections not being returned to the pool in CLI commands

app/lib/redis_configuration.rb
app/models/concerns/redisable.rb
config/initializers/stoplight.rb
lib/mastodon/cli_helper.rb
lib/mastodon/rack_middleware.rb
lib/mastodon/search_cli.rb
lib/mastodon/sidekiq_middleware.rb
spec/services/resolve_account_service_spec.rb

index fc8cf2f80f895955d423c2279b7794fbecc19af2..e14d6c8b6705c3b75fd10d5cdac6d67e2168f119 100644 (file)
@@ -2,12 +2,17 @@
 
 class RedisConfiguration
   class << self
+    def establish_pool(new_pool_size)
+      @pool&.shutdown(&:close)
+      @pool = ConnectionPool.new(size: new_pool_size) { new.connection }
+    end
+
     def with
       pool.with { |redis| yield redis }
     end
 
     def pool
-      @pool ||= ConnectionPool.new(size: pool_size) { new.connection }
+      @pool ||= establish_pool(pool_size)
     end
 
     def pool_size
index cce8efb862acac2271d8f76664429831ba63bb98..8d76b6b82d6f967d7bec6f6c7c05601b5fca2235 100644 (file)
@@ -6,6 +6,6 @@ module Redisable
   private
 
   def redis
-    Thread.current[:redis] ||= RedisConfiguration.new.connection
+    Thread.current[:redis] ||= RedisConfiguration.pool.checkout
   end
 end
index d9ebca76c070d5b9c2f27222a6dee5a504d96b90..8c3c5755ae7eca52d83a15b9b01dbb8944aa3eaa 100644 (file)
@@ -1,4 +1,6 @@
 require 'stoplight'
 
-Stoplight::Light.default_data_store = Stoplight::DataStore::Redis.new(RedisConfiguration.new.connection)
-Stoplight::Light.default_notifiers  = [Stoplight::Notifier::Logger.new(Rails.logger)]
+Rails.application.reloader.to_prepare do
+  Stoplight::Light.default_data_store = Stoplight::DataStore::Redis.new(RedisConfiguration.new.connection)
+  Stoplight::Light.default_notifiers  = [Stoplight::Notifier::Logger.new(Rails.logger)]
+end
index aaee1fa91132c2e79061a153242938062293e0e3..a78a28e2734b2db646bfe7305ff145a24b6425b5 100644 (file)
@@ -19,15 +19,18 @@ module Mastodon
       ProgressBar.create(total: total, format: '%c/%u |%b%i| %e')
     end
 
+    def reset_connection_pools!
+      ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[Rails.env].dup.tap { |config| config['pool'] = options[:concurrency] + 1 })
+      RedisConfiguration.establish_pool(options[:concurrency])
+    end
+
     def parallelize_with_progress(scope)
       if options[:concurrency] < 1
         say('Cannot run with this concurrency setting, must be at least 1', :red)
         exit(1)
       end
 
-      db_config = ActiveRecord::Base.configurations[Rails.env].dup
-      db_config['pool'] = options[:concurrency] + 1
-      ActiveRecord::Base.establish_connection(db_config)
+      reset_connection_pools!
 
       progress  = create_progress_bar(scope.count)
       pool      = Concurrent::FixedThreadPool.new(options[:concurrency])
@@ -52,6 +55,9 @@ module Mastodon
 
               result = ActiveRecord::Base.connection_pool.with_connection do
                 yield(item)
+              ensure
+                RedisConfiguration.pool.checkin if Thread.current[:redis]
+                Thread.current[:redis] = nil
               end
 
               aggregate.increment(result) if result.is_a?(Integer)
index 619a2c36d5b9c5afd33f873061d9bbf1ba37552b..8aa7911fe7edc52a3cbac6b869b008b7dbe2f254 100644 (file)
@@ -19,7 +19,7 @@ class Mastodon::RackMiddleware
   end
 
   def clean_up_redis_socket!
-    Thread.current[:redis]&.close
+    RedisConfiguration.pool.checkin if Thread.current[:redis]
     Thread.current[:redis] = nil
   end
 
index 6ad9d7b6a94f6316403e20dac545d9e48d0854fc..74f980ba11bc321c861e5d0592677e94c9f0dbba 100644 (file)
@@ -59,9 +59,7 @@ module Mastodon
         index.specification.lock!
       end
 
-      db_config = ActiveRecord::Base.configurations[Rails.env].dup
-      db_config['pool'] = options[:concurrency] + 1
-      ActiveRecord::Base.establish_connection(db_config)
+      reset_connection_pools!
 
       pool    = Concurrent::FixedThreadPool.new(options[:concurrency])
       added   = Concurrent::AtomicFixnum.new(0)
@@ -139,6 +137,9 @@ module Mastodon
                 sleep 1
               rescue => e
                 progress.log pastel.red("Error importing #{index}: #{e}")
+              ensure
+                RedisConfiguration.pool.checkin if Thread.current[:redis]
+                Thread.current[:redis] = nil
               end
             end
           end
index 7ec4097dfc79a34b93d178bfc247c6132c5d48ea..c75e8401f5fbc28b62286f01d5c914aedf048394 100644 (file)
@@ -26,7 +26,7 @@ class Mastodon::SidekiqMiddleware
   end
 
   def clean_up_redis_socket!
-    Thread.current[:redis]&.close
+    RedisConfiguration.pool.checkin if Thread.current[:redis]
     Thread.current[:redis] = nil
   end
 
index 7b1e8885ccf4181abd58b0ef1d9123e5c2ed3fd3..8c302e1d863f4dabb5592f0699a56f5c834bd0aa 100644 (file)
@@ -220,6 +220,8 @@ RSpec.describe ResolveAccountService, type: :service do
           return_values << described_class.new.call('foo@ap.example.com')
         rescue ActiveRecord::RecordNotUnique
           fail_occurred = true
+        ensure
+          RedisConfiguration.pool.checkin if Thread.current[:redis]
         end
       end
     end