Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

THREESCALE-8404: Bump redis to 5.2.0 #384

Merged
merged 4 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ gemspec
# implementations (ie. pure Ruby, java, etc).
#
platform :ruby do
gem 'hiredis', '~> 0.6.1'
gem 'hiredis-client'
gem 'yajl-ruby', '~> 1.4.3', require: 'yajl'
gem 'pry-byebug', '~> 3.5.1', groups: [:development]
end
Expand All @@ -24,7 +24,7 @@ group :test do
gem 'mocha', '~> 1.3'
gem 'nokogiri', '~> 1.16.2'
gem 'pkg-config', '~> 1.1.7'
gem 'resque_unit', '~> 0.4.4', source: 'https://rubygems.org'
gem 'resque_unit', '~> 0.4.4'
gem 'test-unit', '~> 3.5'
gem 'resque_spec', '~> 0.17.0'
gem 'timecop', '~> 0.9.1'
Expand Down Expand Up @@ -55,7 +55,8 @@ gem 'daemons', '= 1.2.4'
# Production gems
gem 'rake', '~> 13.0'
gem 'builder', '= 3.2.3'
gem 'resque', '1.27.4'
gem 'redis', '~> 5.0'
gem 'resque', '~> 2.6.0'
gem 'redis-namespace', '~>1.8'
gem 'rack', '~> 2.2.8'
gem 'sinatra', '~> 2.2.4'
Expand All @@ -68,9 +69,5 @@ gem 'async-pool', '~> 0.3.12'
gem 'falcon', '~> 0.35'
gem 'webrick', '~> 1.8'

# Use a patched redis-rb that fixes an issue when trying to connect with
# sentinels and avoids retrying calls when there's a timeout to prevent
# duplicated commands. It's based on version 4.1.3.
gem 'redis', git: 'https://github.com/3scale/redis-rb', branch: 'apisonator'

gem 'dotenv', '~> 2.8.1'
46 changes: 21 additions & 25 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@ GIT
puma (4.3.9)
nio4r (~> 2.0)

GIT
remote: https://github.com/3scale/redis-rb
revision: 7210a9d6cf733fe5a1ad0dd20f5f613167743810
branch: apisonator
specs:
redis (4.1.3)

GIT
remote: https://github.com/3scale/source2swagger
revision: 9a787007577fc58b5822b55720e977cc063057fd
Expand Down Expand Up @@ -60,7 +53,7 @@ GEM
async-io (~> 1.10)
async-pool (~> 0.2)
protocol-redis (~> 0.6.0)
async-rspec (1.13.0)
async-rspec (1.17.0)
rspec (~> 3.0)
rspec-files (~> 1.0)
rspec-memory (~> 1.0)
Expand All @@ -74,11 +67,12 @@ GEM
simplecov (>= 0.7.1, < 1.0.0)
coderay (1.1.3)
concurrent-ruby (1.2.3)
connection_pool (2.4.1)
console (1.23.2)
fiber-annotation
fiber-local
daemons (1.2.4)
diff-lcs (1.3)
diff-lcs (1.5.1)
docile (1.1.5)
dotenv (2.8.1)
dry-initializer (3.0.3)
Expand All @@ -98,7 +92,8 @@ GEM
fiber-annotation (0.2.0)
fiber-local (1.0.0)
gli (2.16.1)
hiredis (0.6.3)
hiredis-client (0.22.1)
redis-client (= 0.22.1)
i18n (1.8.2)
concurrent-ruby (~> 1.0)
json (2.5.1)
Expand All @@ -117,7 +112,7 @@ GEM
minitest (5.18.0)
mocha (1.3.0)
metaclass (~> 0.0.1)
mono_logger (1.1.0)
mono_logger (1.1.2)
multi_json (1.15.0)
mustache (1.0.5)
mustermann (2.0.2)
Expand Down Expand Up @@ -163,14 +158,17 @@ GEM
rack-test (0.8.2)
rack (>= 1.0, < 3)
rake (13.0.1)
redis-namespace (1.10.0)
redis (5.2.0)
redis-client (>= 0.22.0)
redis-client (0.22.1)
connection_pool
redis-namespace (1.11.0)
redis (>= 4)
resque (1.27.4)
resque (2.6.0)
mono_logger (~> 1.0)
multi_json (~> 1.0)
redis-namespace (~> 1.3)
redis-namespace (~> 1.6)
sinatra (>= 0.9.2)
vegas (~> 0.1.2)
resque_spec (0.17.0)
resque (>= 1.19.0)
rspec-core (>= 3.0.0)
Expand All @@ -183,19 +181,19 @@ GEM
rspec-core (~> 3.7.0)
rspec-expectations (~> 3.7.0)
rspec-mocks (~> 3.7.0)
rspec-core (3.7.0)
rspec-core (3.7.1)
rspec-support (~> 3.7.0)
rspec-expectations (3.7.0)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.7.0)
rspec-files (1.0.1)
rspec-files (1.1.3)
rspec (~> 3.0)
rspec-memory (1.0.1)
rspec-memory (1.0.4)
rspec (~> 3.0)
rspec-mocks (3.7.0)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.7.0)
rspec-support (3.7.0)
rspec-support (3.7.1)
rspec_api_documentation (5.1.0)
activesupport (>= 3.0.0)
mustache (~> 1.0, >= 0.99.4)
Expand Down Expand Up @@ -235,8 +233,6 @@ GEM
traces (0.9.1)
tzinfo (1.2.7)
thread_safe (~> 0.1)
vegas (0.1.11)
rack (>= 1.0.0)
webrick (1.8.1)
with_env (1.1.0)
xml-simple (1.1.9)
Expand Down Expand Up @@ -266,7 +262,7 @@ DEPENDENCIES
dotenv (~> 2.8.1)
falcon (~> 0.35)
gli (~> 2.16.1)
hiredis (~> 0.6.1)
hiredis-client
license_finder (~> 7.0)
mocha (~> 1.3)
nokogiri (~> 1.16.2)
Expand All @@ -278,11 +274,11 @@ DEPENDENCIES
rack (~> 2.2.8)
rack-test (= 0.8.2)
rake (~> 13.0)
redis!
redis (~> 5.0)
redis-namespace (~> 1.8)
resque (= 1.27.4)
resque (~> 2.6.0)
resque_spec (~> 0.17.0)
resque_unit (~> 0.4.4)!
resque_unit (~> 0.4.4)
rspec (~> 3.7.0)
rspec_api_documentation (~> 5.0)
sinatra (~> 2.2.4)
Expand Down
13 changes: 7 additions & 6 deletions lib/3scale/backend.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
require_relative 'bundler_shim'

require 'builder'
require 'hiredis'
require 'hiredis-client'

require 'redis'
require 'redis_client/config/timeout' # Monkey patch redis-client

require 'resque'
require 'securerandom'
Expand Down Expand Up @@ -56,18 +57,18 @@
module ThreeScale
module Backend
class << self
def new_rescue_redis
def new_resque_redis
QueueStorage.connection(
environment,
configuration,
)
)
end

def set_rescue_redis
::Resque.redis = new_rescue_redis
def set_resque_redis
::Resque.redis = new_resque_redis
end
end
end
end

ThreeScale::Backend.set_rescue_redis
ThreeScale::Backend.set_resque_redis
4 changes: 2 additions & 2 deletions lib/3scale/backend/alert_limit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class AlertLimit
attr_accessor :service_id, :value

def save
storage.sadd(key_allowed_set(service_id), value.to_i) if valid?
storage.sadd?(key_allowed_set(service_id), value.to_i) if valid?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlledom How did you figure out which calls needed to be updated (from sadd to sadd?) and which are not? Failing tests or smth else?

Cause there are still a lot of occurrences of sadd (for example). I guess it's fine as long as we're not expecting a boolean from them. But how can we make sure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you figure out which calls needed to be updated (from sadd to sadd?) and which are not?

By reading the Changelog, from 4.1.3 to 5.2.0.

For :pipelined and :exists? it's easier because I replaced all occurrences. In the case of :sadd and :sadd? (same for :srem[?]). What I did is check all calls, and add the ? to those where the return value is being used and needs to be a boolean. Then passing tests and making a lot of manual testing in different scenarios. There are not many occurrences of those, so it's doable.

end

def to_hash
Expand All @@ -32,7 +32,7 @@ def self.save(service_id, value)
end

def self.delete(service_id, value)
storage.srem(key_allowed_set(service_id), value.to_i) if valid_value?(value)
storage.srem?(key_allowed_set(service_id), value.to_i) if valid_value?(value)
end

def self.valid_value?(value)
Expand Down
12 changes: 6 additions & 6 deletions lib/3scale/backend/alerts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@ def update_utilization(service_id, app_id, max_utilization, max_record, timestam

keys = alert_keys(service_id, app_id, discrete)

already_alerted, allowed = storage.pipelined do
storage.get(keys[:already_notified])
storage.sismember(keys[:allowed], discrete)
already_alerted, allowed = storage.pipelined do |pipeline|
pipeline.get(keys[:already_notified])
pipeline.sismember(keys[:allowed], discrete)
end

if already_alerted.nil? && allowed && discrete.to_i > 0
next_id, _ = storage.pipelined do
storage.incr(keys[:current_id])
storage.setex(keys[:already_notified], ALERT_TTL, "1")
next_id, _ = storage.pipelined do |pipeline|
pipeline.incr(keys[:current_id])
pipeline.setex(keys[:already_notified], ALERT_TTL, "1")
end

alert = { :id => next_id,
Expand Down
40 changes: 20 additions & 20 deletions lib/3scale/backend/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def extract_id!(service_id, app_id, user_key)
end

def exists?(service_id, id)
storage.exists(storage_key(service_id, id, :state))
storage.exists?(storage_key(service_id, id, :state))
end
memoize :exists?

Expand All @@ -102,9 +102,9 @@ def delete(service_id, id)
end

def delete_data(service_id, id)
storage.pipelined do
delete_set(service_id, id)
delete_attributes(service_id, id)
storage.pipelined do |pipeline|
delete_set(pipeline, service_id, id)
delete_attributes(pipeline, service_id, id)
end
end

Expand Down Expand Up @@ -137,12 +137,12 @@ def id_by_key_storage_key(service_id, key)
encode_key("application/service_id:#{service_id}/key:#{key}/id")
end

def delete_set(service_id, id)
storage.srem(applications_set_key(service_id), id)
def delete_set(client, service_id, id)
client.srem(applications_set_key(service_id), id)
end

def delete_attributes(service_id, id)
storage.del(
def delete_attributes(client, service_id, id)
client.del(
ATTRIBUTES.map do |f|
storage_key(service_id, id, f)
end
Expand All @@ -166,9 +166,9 @@ def with_app_id_from_params(service_id, app_id, user_key)
def save
raise ApplicationHasNoState.new(id) if !state

storage.pipelined do
persist_attributes
persist_set
storage.pipelined do |pipeline|
persist_attributes(pipeline)
persist_set(pipeline)
end

self.class.clear_cache(service_id, id)
Expand Down Expand Up @@ -260,7 +260,7 @@ def create_key(value = nil)
def delete_key(value)
db_key = storage_key(:keys)
invalidate_cache([:smembers, :scard, :sismember], db_key)
storage.srem(db_key, value)
storage.srem?(db_key, value)
end

def has_keys?
Expand All @@ -279,7 +279,7 @@ def has_key?(value)
db_key = storage_key(:keys)
key = Memoizer.build_key(self.class, :sismember, db_key, value)
Memoizer.memoize_block(key) do
storage.sismember(db_key, value)
storage.sismember(db_key, value.to_s)
end
end

Expand Down Expand Up @@ -319,15 +319,15 @@ def has_referrer_filters?

private

def persist_attributes
storage.set(storage_key(:state), state.to_s) if state
storage.set(storage_key(:plan_id), plan_id) if plan_id
storage.set(storage_key(:plan_name), plan_name) if plan_name
storage.set(storage_key(:redirect_url), redirect_url) if redirect_url
def persist_attributes(client)
client.set(storage_key(:state), state.to_s) if state
client.set(storage_key(:plan_id), plan_id) if plan_id
client.set(storage_key(:plan_name), plan_name) if plan_name
client.set(storage_key(:redirect_url), redirect_url) if redirect_url
end

def persist_set
storage.sadd(applications_set_key(service_id), id)
def persist_set(client)
client.sadd(applications_set_key(service_id), id)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/3scale/backend/application_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ def self.generate(applications)
private

def self.first_traffic(service_id, application_id)
if storage.sadd(Stats::Keys.set_of_apps_with_traffic(service_id),
encode_key(application_id))
if storage.sadd?(Stats::Keys.set_of_apps_with_traffic(service_id),
encode_key(application_id))
EventStorage.store(:first_traffic,
{ service_id: service_id,
application_id: application_id,
Expand Down
10 changes: 5 additions & 5 deletions lib/3scale/backend/event_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ def delete(id)
(id > 0) ? storage.zremrangebyscore(events_queue_key, id, id) : 0
end

def size
storage.zcard(events_queue_key)
def size(strg = storage)
strg.zcard(events_queue_key)
end

def ping_if_not_empty
Expand Down Expand Up @@ -90,9 +90,9 @@ def events_hook_uri
def pending_ping?
## the queue is not empty and more than timeout has passed
## since the front-end was notified
events_set_size, can_ping = storage.pipelined do
size
storage.set(events_ping_key, '1'.freeze, ex: PING_TTL, nx: true)
events_set_size, can_ping = storage.pipelined do |pipeline|
size(pipeline)
pipeline.set(events_ping_key, '1'.freeze, ex: PING_TTL, nx: true)
end

can_ping && events_set_size > 0
Expand Down
3 changes: 3 additions & 0 deletions lib/3scale/backend/job_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ def try_pop_from_queue(max=nil)
def wait_pop_from_queue
queue, encoded = @redis.blpop(*@queues, timeout: @fetch_timeout)
[queue, [encoded]] if encoded
rescue RedisClient::ReadTimeoutError => _e
# Ignore this exception, this happens because of a bug on redis-rb, when connecting to sentinels.
# Check: https://github.com/redis/redis-rb/issues/1279
end

def decode_job(queue, encoded_job)
Expand Down
Loading