From 922837dc96154b0455a4cf660c3f8369c65aacb4 Mon Sep 17 00:00:00 2001 From: Jean byroot Boussier Date: Sat, 4 Mar 2023 16:38:28 +0100 Subject: [PATCH] Upgrade to latest redis-rb 4.x and fix deprecations (#23616) Co-authored-by: Jean Boussier --- Gemfile.lock | 2 +- app/lib/feed_manager.rb | 20 +++++++++---------- .../follow_recommendation_suppression.rb | 4 ++-- app/services/batched_remove_status_service.rb | 18 ++++++++--------- .../follow_recommendations_scheduler.rb | 13 +++++------- config/environments/development.rb | 2 ++ config/environments/test.rb | 2 ++ config/initializers/redis.rb | 1 + .../20170920032311_fix_reblogs_in_feeds.rb | 2 +- ...00407202420_migrate_unavailable_inboxes.rb | 5 +++-- lib/mastodon/feeds_cli.rb | 6 +----- 11 files changed, 37 insertions(+), 38 deletions(-) create mode 100644 config/initializers/redis.rb 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