Upgrade to latest redis-rb 4.x and fix deprecations (#23616)

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

11 files changed, 37 insertions, 38 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index 51cf8147b..b8b094325 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -558,7 +558,7 @@ GEM
     rdf-normalize (0.5.1)
       rdf (~> 3.2)
     redcarpet (3.6.0)
-    redis (4.5.1)
+    redis (4.8.1)
     redis-namespace (1.10.0)
       redis (>= 4)
     redlock (1.3.2)
diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb
index 8d7540e0f..7dda6b185 100644
--- a/app/lib/feed_manager.rb
+++ b/app/lib/feed_manager.rb
@@ -273,27 +273,27 @@ class FeedManager
   def clean_feeds!(type, ids)
     reblogged_id_sets = {}
 
-    redis.pipelined do
+    redis.pipelined do |pipeline|
       ids.each do |feed_id|
-        redis.del(key(type, feed_id))
         reblog_key = key(type, feed_id, 'reblogs')
         # We collect a future for this: we don't block while getting
         # it, but we can iterate over it later.
-        reblogged_id_sets[feed_id] = redis.zrange(reblog_key, 0, -1)
-        redis.del(reblog_key)
+        reblogged_id_sets[feed_id] = pipeline.zrange(reblog_key, 0, -1)
+        pipeline.del(key(type, feed_id), reblog_key)
       end
     end
 
     # Remove all of the reblog tracking keys we just removed the
     # references to.
-    redis.pipelined do
-      reblogged_id_sets.each do |feed_id, future|
-        future.value.each do |reblogged_id|
-          reblog_set_key = key(type, feed_id, "reblogs:#{reblogged_id}")
-          redis.del(reblog_set_key)
-        end
+    keys_to_delete = reblogged_id_sets.flat_map do |feed_id, future|
+      future.value.map do |reblogged_id|
+        key(type, feed_id, "reblogs:#{reblogged_id}")
       end
     end
+
+    redis.del(keys_to_delete) unless keys_to_delete.empty?
+
+    nil
   end
 
   private
diff --git a/app/models/follow_recommendation_suppression.rb b/app/models/follow_recommendation_suppression.rb
index a9dbbfc18..e261a2fe3 100644
--- a/app/models/follow_recommendation_suppression.rb
+++ b/app/models/follow_recommendation_suppression.rb
@@ -20,9 +20,9 @@ class FollowRecommendationSuppression < ApplicationRecord
   private
 
   def remove_follow_recommendations
-    redis.pipelined do
+    redis.pipelined do |pipeline|
       I18n.available_locales.each do |locale|
-        redis.zrem("follow_recommendations:#{locale}", account_id)
+        pipeline.zrem("follow_recommendations:#{locale}", account_id)
       end
     end
   end
diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb
index 54e5f10a4..7e9b67126 100644
--- a/app/services/batched_remove_status_service.rb
+++ b/app/services/batched_remove_status_service.rb
@@ -45,9 +45,9 @@ class BatchedRemoveStatusService < BaseService
 
     # Cannot be batched
     @status_id_cutoff = Mastodon::Snowflake.id_at(2.weeks.ago)
-    redis.pipelined do
+    redis.pipelined do |pipeline|
       statuses.each do |status|
-        unpush_from_public_timelines(status)
+        unpush_from_public_timelines(status, pipeline)
       end
     end
   end
@@ -70,22 +70,22 @@ class BatchedRemoveStatusService < BaseService
     end
   end
 
-  def unpush_from_public_timelines(status)
+  def unpush_from_public_timelines(status, pipeline)
     return unless status.public_visibility? && status.id > @status_id_cutoff
 
     payload = Oj.dump(event: :delete, payload: status.id.to_s)
 
-    redis.publish('timeline:public', payload)
-    redis.publish(status.local? ? 'timeline:public:local' : 'timeline:public:remote', payload)
+    pipeline.publish('timeline:public', payload)
+    pipeline.publish(status.local? ? 'timeline:public:local' : 'timeline:public:remote', payload)
 
     if status.media_attachments.any?
-      redis.publish('timeline:public:media', payload)
-      redis.publish(status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', payload)
+      pipeline.publish('timeline:public:media', payload)
+      pipeline.publish(status.local? ? 'timeline:public:local:media' : 'timeline:public:remote:media', payload)
     end
 
     status.tags.map { |tag| tag.name.mb_chars.downcase }.each do |hashtag|
-      redis.publish("timeline:hashtag:#{hashtag}", payload)
-      redis.publish("timeline:hashtag:#{hashtag}:local", payload) if status.local?
+      pipeline.publish("timeline:hashtag:#{hashtag}", payload)
+      pipeline.publish("timeline:hashtag:#{hashtag}:local", payload) if status.local?
     end
   end
 end
diff --git a/app/workers/scheduler/follow_recommendations_scheduler.rb b/app/workers/scheduler/follow_recommendations_scheduler.rb
index 04008a9d9..17cf3f2cc 100644
--- a/app/workers/scheduler/follow_recommendations_scheduler.rb
+++ b/app/workers/scheduler/follow_recommendations_scheduler.rb
@@ -20,7 +20,7 @@ class Scheduler::FollowRecommendationsScheduler
 
     Trends.available_locales.each do |locale|
       recommendations = if AccountSummary.safe.filtered.localized(locale).exists? # We can skip the work if no accounts with that language exist
-                          FollowRecommendation.localized(locale).order(rank: :desc).limit(SET_SIZE).map { |recommendation| [recommendation.account_id, recommendation.rank] }
+                          FollowRecommendation.localized(locale).order(rank: :desc).limit(SET_SIZE).map { |recommendation| [recommendation.rank, recommendation.account_id] }
                         else
                           []
                         end
@@ -33,14 +33,14 @@ class Scheduler::FollowRecommendationsScheduler
 
         # Language-specific results should be above language-agnostic ones,
         # otherwise language-agnostic ones will always overshadow them
-        recommendations.map! { |(account_id, rank)| [account_id, rank + max_fallback_rank] }
+        recommendations.map! { |(rank, account_id)| [rank + max_fallback_rank, account_id] }
 
         added = 0
 
         fallback_recommendations.each do |recommendation|
-          next if recommendations.any? { |(account_id, _)| account_id == recommendation.account_id }
+          next if recommendations.any? { |(_, account_id)| account_id == recommendation.account_id }
 
-          recommendations << [recommendation.account_id, recommendation.rank]
+          recommendations << [recommendation.rank, recommendation.account_id]
           added += 1
 
           break if added >= missing
@@ -49,10 +49,7 @@ class Scheduler::FollowRecommendationsScheduler
 
       redis.multi do |multi|
         multi.del(key(locale))
-
-        recommendations.each do |(account_id, rank)|
-          multi.zadd(key(locale), rank, account_id)
-        end
+        multi.zadd(key(locale), recommendations)
       end
     end
   end
diff --git a/config/environments/development.rb b/config/environments/development.rb
index de8762ff7..29b17a350 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -87,6 +87,8 @@ Rails.application.configure do
   config.x.otp_secret = ENV.fetch('OTP_SECRET', '1fc2b87989afa6351912abeebe31ffc5c476ead9bf8b3d74cbc4a302c7b69a45b40b1bbef3506ddad73e942e15ed5ca4b402bf9a66423626051104f4b5f05109')
 end
 
+Redis.raise_deprecations = true
+
 ActiveRecordQueryTrace.enabled = ENV['QUERY_TRACE_ENABLED'] == 'true'
 
 module PrivateAddressCheck
diff --git a/config/environments/test.rb b/config/environments/test.rb
index ef3cb2e48..9cbf31e8d 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -73,3 +73,5 @@ end
 
 # Catch serialization warnings early
 Sidekiq.strict_args!
+
+Redis.raise_deprecations = true
diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb
new file mode 100644
index 000000000..f2bbd1e45
--- /dev/null
+++ b/config/initializers/redis.rb
@@ -0,0 +1 @@
+Redis.sadd_returns_boolean = false
diff --git a/db/migrate/20170920032311_fix_reblogs_in_feeds.rb b/db/migrate/20170920032311_fix_reblogs_in_feeds.rb
index 4ab68e8f3..7e2db0ff3 100644
--- a/db/migrate/20170920032311_fix_reblogs_in_feeds.rb
+++ b/db/migrate/20170920032311_fix_reblogs_in_feeds.rb
@@ -1,6 +1,6 @@
 class FixReblogsInFeeds < ActiveRecord::Migration[5.1]
   def up
-    redis = Redis.current
+    redis = RedisConfiguration.pool.checkout
     fm = FeedManager.instance
 
     # Old scheme:
diff --git a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb
index 92a3acb5d..8f9c68794 100644
--- a/db/migrate/20200407202420_migrate_unavailable_inboxes.rb
+++ b/db/migrate/20200407202420_migrate_unavailable_inboxes.rb
@@ -2,7 +2,8 @@ class MigrateUnavailableInboxes < ActiveRecord::Migration[5.2]
   disable_ddl_transaction!
 
   def up
-    urls = Redis.current.smembers('unavailable_inboxes')
+    redis = RedisConfiguration.pool.checkout
+    urls = redis.smembers('unavailable_inboxes')
 
     hosts = urls.map do |url|
       Addressable::URI.parse(url).normalized_host
@@ -14,7 +15,7 @@ class MigrateUnavailableInboxes < ActiveRecord::Migration[5.2]
       UnavailableDomain.create(domain: host)
     end
 
-    Redis.current.del(*(['unavailable_inboxes'] + Redis.current.keys('exhausted_deliveries:*')))
+    redis.del(*(['unavailable_inboxes'] + redis.keys('exhausted_deliveries:*')))
   end
 
   def down; end
diff --git a/lib/mastodon/feeds_cli.rb b/lib/mastodon/feeds_cli.rb
index 428d63a44..fcfb48740 100644
--- a/lib/mastodon/feeds_cli.rb
+++ b/lib/mastodon/feeds_cli.rb
@@ -53,11 +53,7 @@ module Mastodon
     desc 'clear', 'Remove all home and list feeds from Redis'
     def clear
       keys = redis.keys('feed:*')
-
-      redis.pipelined do
-        keys.each { |key| redis.del(key) }
-      end
-
+      redis.del(keys)
       say('OK', :green)
     end
   end