From f610ac46a511be3b74eed3885a3808aacc468485 Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Sat, 14 Jan 2023 21:18:18 +0100 Subject: [PATCH 1/7] Update ruby base version and dependencies redis-rb 4 and 5 changed how the reconnect works --- .github/workflows/test.yml | 4 ++-- Gemfile.lock | 38 +++++++++++++++++-------------- LICENSE | 2 +- gcra.gemspec | 4 ++-- lib/gcra/redis_store.rb | 19 ++++++++-------- lib/gcra/version.rb | 2 +- spec/lib/gcra/redis_store_spec.rb | 16 ++++++------- 7 files changed, 45 insertions(+), 40 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f770500..1836146 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: ['2.5', '2.6', '2.7', '3.0', '3.1'] + ruby: ['2.7', '3.0', '3.1'] redis-version: [5, 6] steps: @@ -33,7 +33,7 @@ jobs: - name: Install dependencies run: | - gem install bundler -v '1.17.3' --no-document + gem install bundler -v '2.4.3' --no-document bundle install --jobs 4 --retry 3 - name: Run Tests diff --git a/Gemfile.lock b/Gemfile.lock index 4d5544b..f9f3afe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,34 +1,38 @@ PATH remote: . specs: - gcra (1.2.1) + gcra (1.3.0) GEM remote: https://rubygems.org/ specs: - diff-lcs (1.2.5) - redis (3.3.3) - rspec (3.5.0) - rspec-core (~> 3.5.0) - rspec-expectations (~> 3.5.0) - rspec-mocks (~> 3.5.0) - rspec-core (3.5.4) - rspec-support (~> 3.5.0) - rspec-expectations (3.5.0) + connection_pool (2.3.0) + diff-lcs (1.5.0) + redis (5.0.5) + redis-client (>= 0.9.0) + redis-client (0.12.0) + connection_pool + rspec (3.12.0) + rspec-core (~> 3.12.0) + rspec-expectations (~> 3.12.0) + rspec-mocks (~> 3.12.0) + rspec-core (3.12.0) + rspec-support (~> 3.12.0) + rspec-expectations (3.12.2) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.5.0) - rspec-mocks (3.5.0) + rspec-support (~> 3.12.0) + rspec-mocks (3.12.2) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.5.0) - rspec-support (3.5.0) + rspec-support (~> 3.12.0) + rspec-support (3.12.0) PLATFORMS ruby DEPENDENCIES gcra! - redis (~> 3.3) - rspec (~> 3.5) + redis (~> 5.0.5) + rspec (~> 3.12) BUNDLED WITH - 1.17.3 + 2.4.2 diff --git a/LICENSE b/LICENSE index 5c72e91..b0177f7 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2022 viafintech GmbH +Copyright (c) 2023 viafintech GmbH Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/gcra.gemspec b/gcra.gemspec index 03f9349..e4279ed 100644 --- a/gcra.gemspec +++ b/gcra.gemspec @@ -17,6 +17,6 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^spec/}) spec.require_paths = ['lib'] - spec.add_development_dependency 'rspec', '~> 3.5' - spec.add_development_dependency 'redis', '~> 3.3' + spec.add_development_dependency 'rspec', '~> 3.12' + spec.add_development_dependency 'redis', '~> 5.0.5' end diff --git a/lib/gcra/redis_store.rb b/lib/gcra/redis_store.rb index 1999c13..d523884 100644 --- a/lib/gcra/redis_store.rb +++ b/lib/gcra/redis_store.rb @@ -18,8 +18,6 @@ class RedisStore CAS_SCRIPT_MISSING_KEY_RESPONSE = 'key does not exist'.freeze SCRIPT_NOT_IN_CACHE_RESPONSE = 'NOSCRIPT No matching script. Please use EVAL.'.freeze - CONNECTED_TO_READONLY = "READONLY You can't write against a read only slave.".freeze - def initialize(redis, key_prefix, options = {}) @redis = redis @key_prefix = key_prefix @@ -51,9 +49,9 @@ def set_if_not_exists_with_ttl(key, value, ttl_nano) begin ttl_milli = calculate_ttl_milli(ttl_nano) @redis.set(full_key, value, nx: true, px: ttl_milli) - rescue Redis::CommandError => e - if e.message == CONNECTED_TO_READONLY && @reconnect_on_readonly && !retried - @redis.client.reconnect + rescue Redis::ReadOnlyError => e + if @reconnect_on_readonly && !retried + @redis.close retried = true retry end @@ -70,6 +68,13 @@ def compare_and_set_with_ttl(key, old_value, new_value, ttl_nano) begin ttl_milli = calculate_ttl_milli(ttl_nano) swapped = @redis.evalsha(CAS_SHA, keys: [full_key], argv: [old_value, new_value, ttl_milli]) + rescue Redis::ReadOnlyError => e + if @reconnect_on_readonly && !retried + @redis.close + retried = true + retry + end + raise rescue Redis::CommandError => e if e.message == CAS_SCRIPT_MISSING_KEY_RESPONSE return false @@ -77,10 +82,6 @@ def compare_and_set_with_ttl(key, old_value, new_value, ttl_nano) @redis.script('load', CAS_SCRIPT) retried = true retry - elsif e.message == CONNECTED_TO_READONLY && @reconnect_on_readonly && !retried - @redis.client.reconnect - retried = true - retry end raise end diff --git a/lib/gcra/version.rb b/lib/gcra/version.rb index a9bb257..a0deefa 100644 --- a/lib/gcra/version.rb +++ b/lib/gcra/version.rb @@ -1,3 +1,3 @@ module GCRA - VERSION = '1.2.1'.freeze + VERSION = '1.3.0'.freeze end diff --git a/spec/lib/gcra/redis_store_spec.rb b/spec/lib/gcra/redis_store_spec.rb index 3486859..2449da5 100644 --- a/spec/lib/gcra/redis_store_spec.rb +++ b/spec/lib/gcra/redis_store_spec.rb @@ -69,7 +69,7 @@ def cleanup_redis end it 'with a readonly host and no readonly configured' do - exception = Redis::CommandError.new(GCRA::RedisStore::CONNECTED_TO_READONLY) + exception = Redis::ReadOnlyError.new expect(redis) .to receive(:set) @@ -89,10 +89,10 @@ def cleanup_redis end it 'with a readonly host' do - exception = Redis::CommandError.new(GCRA::RedisStore::CONNECTED_TO_READONLY) + exception = Redis::ReadOnlyError.new - expect(redis.client) - .to receive(:reconnect) + expect(redis) + .to receive(:close) .and_call_original expect(redis) @@ -201,7 +201,7 @@ def cleanup_redis context 'with reconnect on readonly not configured' do it 'raises an error when the request is executed against a readonly host' do - exception = Redis::CommandError.new(GCRA::RedisStore::CONNECTED_TO_READONLY) + exception = Redis::ReadOnlyError.new expect(redis) .to receive(:evalsha) @@ -231,10 +231,10 @@ def cleanup_redis end it 'attempts a reconnect once and then executes evalsha again' do - exception = Redis::CommandError.new(GCRA::RedisStore::CONNECTED_TO_READONLY) + exception = Redis::ReadOnlyError.new - expect(redis.client) - .to receive(:reconnect) + expect(redis) + .to receive(:close) .and_call_original expect(redis) From 00330e63ca61dd80e01473702dbd459050ccbb17 Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Fri, 29 Mar 2024 09:03:22 +0100 Subject: [PATCH 2/7] Add redis 7 to test matrix --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1836146..ef07385 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: fail-fast: false matrix: ruby: ['2.7', '3.0', '3.1'] - redis-version: [5, 6] + redis-version: [5, 6, 7] steps: - name: Checkout code From 248dcd6e0f884e698c40666415c52c56a872f7e1 Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Fri, 29 Mar 2024 09:05:49 +0100 Subject: [PATCH 3/7] Remove ruby 2.7 and add ruby 3.2 to test matrix Also update bundler version --- .github/workflows/test.yml | 4 ++-- Gemfile.lock | 2 +- LICENSE | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ef07385..3d961e5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: ['2.7', '3.0', '3.1'] + ruby: ['3.0', '3.1', '3.2'] redis-version: [5, 6, 7] steps: @@ -33,7 +33,7 @@ jobs: - name: Install dependencies run: | - gem install bundler -v '2.4.3' --no-document + gem install bundler -v '2.5.7' --no-document bundle install --jobs 4 --retry 3 - name: Run Tests diff --git a/Gemfile.lock b/Gemfile.lock index f9f3afe..1260a95 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -35,4 +35,4 @@ DEPENDENCIES rspec (~> 3.12) BUNDLED WITH - 2.4.2 + 2.5.7 diff --git a/LICENSE b/LICENSE index b0177f7..9f48868 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2023 viafintech GmbH +Copyright (c) 2024 viafintech GmbH Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal From 311b2ae82d31af72efb744922e7338d268c75839 Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Fri, 29 Mar 2024 09:08:15 +0100 Subject: [PATCH 4/7] Update redis version in dev dependencies --- Gemfile.lock | 10 +++++----- gcra.gemspec | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1260a95..85fb041 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,11 +6,11 @@ PATH GEM remote: https://rubygems.org/ specs: - connection_pool (2.3.0) + connection_pool (2.4.1) diff-lcs (1.5.0) - redis (5.0.5) - redis-client (>= 0.9.0) - redis-client (0.12.0) + redis (5.1.0) + redis-client (>= 0.17.0) + redis-client (0.21.1) connection_pool rspec (3.12.0) rspec-core (~> 3.12.0) @@ -31,7 +31,7 @@ PLATFORMS DEPENDENCIES gcra! - redis (~> 5.0.5) + redis (~> 5.1) rspec (~> 3.12) BUNDLED WITH diff --git a/gcra.gemspec b/gcra.gemspec index e4279ed..0ced748 100644 --- a/gcra.gemspec +++ b/gcra.gemspec @@ -18,5 +18,5 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_development_dependency 'rspec', '~> 3.12' - spec.add_development_dependency 'redis', '~> 5.0.5' + spec.add_development_dependency 'redis', '~> 5.1' end From a47007e2431a941c8f3661ad91ea650cb465aeac Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Fri, 29 Mar 2024 11:12:39 +0100 Subject: [PATCH 5/7] Deal with error message format change in redis 5.1 --- lib/gcra/redis_store.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/gcra/redis_store.rb b/lib/gcra/redis_store.rb index d523884..21ae9ab 100644 --- a/lib/gcra/redis_store.rb +++ b/lib/gcra/redis_store.rb @@ -15,8 +15,10 @@ class RedisStore # Digest::SHA1.hexdigest(CAS_SCRIPT) CAS_SHA = "89118e702230c0d65969c5fc557a6e942a2f4d31".freeze - CAS_SCRIPT_MISSING_KEY_RESPONSE = 'key does not exist'.freeze - SCRIPT_NOT_IN_CACHE_RESPONSE = 'NOSCRIPT No matching script. Please use EVAL.'.freeze + CAS_SCRIPT_MISSING_KEY_RESPONSE_PATTERN = Regexp.new('^key does not exist') + SCRIPT_NOT_IN_CACHE_RESPONSE_PATTERN = Regexp.new( + '^NOSCRIPT No matching script. Please use EVAL.', + ) def initialize(redis, key_prefix, options = {}) @redis = redis @@ -76,9 +78,9 @@ def compare_and_set_with_ttl(key, old_value, new_value, ttl_nano) end raise rescue Redis::CommandError => e - if e.message == CAS_SCRIPT_MISSING_KEY_RESPONSE + if e.message =~ CAS_SCRIPT_MISSING_KEY_RESPONSE_PATTERN return false - elsif e.message == SCRIPT_NOT_IN_CACHE_RESPONSE && !retried + elsif e.message =~ SCRIPT_NOT_IN_CACHE_RESPONSE_PATTERN && !retried @redis.script('load', CAS_SCRIPT) retried = true retry From d1040c1a00f3df3eb86b31a81b834692ba08c664 Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Fri, 29 Mar 2024 11:25:39 +0100 Subject: [PATCH 6/7] Also check behaviour for redis 4 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3d961e5..9c13f9b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: fail-fast: false matrix: ruby: ['3.0', '3.1', '3.2'] - redis-version: [5, 6, 7] + redis-version: [4, 5, 6, 7] steps: - name: Checkout code From 53f142772b15bc59cb2dd8a66cdb11df042182c7 Mon Sep 17 00:00:00 2001 From: Tobias Schoknecht Date: Fri, 29 Mar 2024 11:27:10 +0100 Subject: [PATCH 7/7] Add ruby 3.3 to tested versions --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9c13f9b..53648e8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: ['3.0', '3.1', '3.2'] + ruby: ['3.0', '3.1', '3.2', '3.3'] redis-version: [4, 5, 6, 7] steps: