From 975a752004a1c41318446e6291e720b2cd756e89 Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Thu, 11 Nov 2021 15:56:17 -0800 Subject: [PATCH 1/9] Adding retry mechanism for execute method --- lib/makara/connection_wrapper.rb | 34 +++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/makara/connection_wrapper.rb b/lib/makara/connection_wrapper.rb index e63b2eb6..9b9a3048 100644 --- a/lib/makara/connection_wrapper.rb +++ b/lib/makara/connection_wrapper.rb @@ -63,10 +63,10 @@ def _makara_connected? false end - def _makara_connection + def _makara_connection(force_new = false) current = @connection - if current + if current && !force_new current else # blacklisted connection or initial error new_connection = @proxy.graceful_connection_for(@config) @@ -90,7 +90,35 @@ def execute(*args) end end - _makara_connection.execute(*args) + if @proxy.config.key?(:retry_exceptions) && (retry_exceptions = @proxy.config[:retry_exceptions]).present? + _execute_with_retry(retry_exceptions, *args) + else + _makara_connection.execute(*args) + end + end + + MAX_RETRY_ATTEMPTS = 10 + + # Attempt to retry the execution by re-establishing the connection + def _execute_with_retry(retry_exceptions, *args) + retry_attempts = retry_exceptions.inject({}).map { |memo, retry_exception| memo[retry_exception['name']] = 0 } + begin + _makara_connection.execute(*args) + rescue Exception => actual_exception + retry_exceptions.each do |retry_exception| + if actual_exception.class.to_s == retry_exception + _makara_connection(true) # Force new connection to re-try. + sleep retry_exception['time_between_retries_in_seconds'] || 0.1 + retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) + + if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS + retry + end + end + end + + raise actual_exception + end end # we want to forward all private methods, since we could have kicked out from a private scenario From 923bc43bbfd6e844a880ee49f3e193081f956c9d Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 15:33:55 -0800 Subject: [PATCH 2/9] Fixing the retry mechanism and update gitignore --- .gitignore | 1 + lib/makara/connection_wrapper.rb | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index d930387d..002af981 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ spec/reports test/tmp test/version_tmp tmp +.idea diff --git a/lib/makara/connection_wrapper.rb b/lib/makara/connection_wrapper.rb index 9b9a3048..1e5aa470 100644 --- a/lib/makara/connection_wrapper.rb +++ b/lib/makara/connection_wrapper.rb @@ -103,6 +103,7 @@ def execute(*args) def _execute_with_retry(retry_exceptions, *args) retry_attempts = retry_exceptions.inject({}).map { |memo, retry_exception| memo[retry_exception['name']] = 0 } begin + should_retry = false _makara_connection.execute(*args) rescue Exception => actual_exception retry_exceptions.each do |retry_exception| @@ -112,11 +113,11 @@ def _execute_with_retry(retry_exceptions, *args) retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS - retry + should_retry = true end end end - + retry if should_retry raise actual_exception end end From a1e6ee0343c90df025214e642f2bf9ab635c5696 Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 15:45:27 -0800 Subject: [PATCH 3/9] See OPS-177 : Fixing inject syntax --- lib/makara/connection_wrapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/makara/connection_wrapper.rb b/lib/makara/connection_wrapper.rb index 1e5aa470..2ca5cf54 100644 --- a/lib/makara/connection_wrapper.rb +++ b/lib/makara/connection_wrapper.rb @@ -101,7 +101,7 @@ def execute(*args) # Attempt to retry the execution by re-establishing the connection def _execute_with_retry(retry_exceptions, *args) - retry_attempts = retry_exceptions.inject({}).map { |memo, retry_exception| memo[retry_exception['name']] = 0 } + retry_attempts = retry_exceptions.inject({}) { |memo, retry_exception| memo[retry_exception['name']] = 0; memo } begin should_retry = false _makara_connection.execute(*args) From 4459e8135fbf08cc7b93f843a1cd4b3818f7be3c Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 16:02:02 -0800 Subject: [PATCH 4/9] See OPS-177 : Fixing logical error in retry mechanism --- lib/makara/connection_wrapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/makara/connection_wrapper.rb b/lib/makara/connection_wrapper.rb index 2ca5cf54..02a20aab 100644 --- a/lib/makara/connection_wrapper.rb +++ b/lib/makara/connection_wrapper.rb @@ -107,7 +107,7 @@ def _execute_with_retry(retry_exceptions, *args) _makara_connection.execute(*args) rescue Exception => actual_exception retry_exceptions.each do |retry_exception| - if actual_exception.class.to_s == retry_exception + if actual_exception.class.to_s == retry_exception['name'] _makara_connection(true) # Force new connection to re-try. sleep retry_exception['time_between_retries_in_seconds'] || 0.1 retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) From 50f0393eafffe9dbf3511b5de642c98d4e3537a1 Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 16:43:41 -0800 Subject: [PATCH 5/9] See OPS-177 : Implementing the retry mechanism at the appropriate place --- lib/makara/connection_wrapper.rb | 31 +-------------------- lib/makara/proxy.rb | 46 ++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/lib/makara/connection_wrapper.rb b/lib/makara/connection_wrapper.rb index 02a20aab..c212c849 100644 --- a/lib/makara/connection_wrapper.rb +++ b/lib/makara/connection_wrapper.rb @@ -90,36 +90,7 @@ def execute(*args) end end - if @proxy.config.key?(:retry_exceptions) && (retry_exceptions = @proxy.config[:retry_exceptions]).present? - _execute_with_retry(retry_exceptions, *args) - else - _makara_connection.execute(*args) - end - end - - MAX_RETRY_ATTEMPTS = 10 - - # Attempt to retry the execution by re-establishing the connection - def _execute_with_retry(retry_exceptions, *args) - retry_attempts = retry_exceptions.inject({}) { |memo, retry_exception| memo[retry_exception['name']] = 0; memo } - begin - should_retry = false - _makara_connection.execute(*args) - rescue Exception => actual_exception - retry_exceptions.each do |retry_exception| - if actual_exception.class.to_s == retry_exception['name'] - _makara_connection(true) # Force new connection to re-try. - sleep retry_exception['time_between_retries_in_seconds'] || 0.1 - retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) - - if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS - should_retry = true - end - end - end - retry if should_retry - raise actual_exception - end + _makara_connection.execute(*args) end # we want to forward all private methods, since we could have kicked out from a private scenario diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index e3e661fb..3f1ed905 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -18,14 +18,20 @@ class Proxy < ::SimpleDelegator self.hijack_methods = [] class << self + MAX_RETRY_ATTEMPTS = 10 + def hijack_method(*method_names) self.hijack_methods = self.hijack_methods || [] self.hijack_methods |= method_names method_names.each do |method_name| define_method method_name do |*args, &block| - appropriate_connection(method_name, args) do |con| - con.send(method_name, *args, &block) + + makara_config = @config.with_indifferent_access[:makara] + if makara_config.key?(:retry_exceptions) && (retry_exceptions = makara_config[:retry_exceptions]).present? + _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, method_name) + else + _execute_with_connection(args, block, method_name) end end end @@ -38,6 +44,42 @@ def send_to_all(*method_names) end end end + + private + + def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, method_name) + begin + should_retry = false + potentially_stale_connection = nil + + appropriate_connection(method_name, args) do |con| + con.send(method_name, *args, &block) + potentially_stale_connection = con + end + + rescue Exception => actual_exception + retry_attempts = retry_exceptions.inject({}) { |memo, retry_exception| memo[retry_exception['name']] = 0; memo } + retry_exceptions.each do |retry_exception| + if actual_exception.class.to_s == retry_exception['name'] + potentially_stale_connection.disconnect! + sleep retry_exception['time_between_retries_in_seconds'] || 0.1 + retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) + + if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS + should_retry = true + end + end + end + retry if should_retry + raise actual_exception + end + end + + def _execute_with_connection(args, block, method_name) + appropriate_connection(method_name, args) do |con| + con.send(method_name, *args, &block) + end + end end From 64337e4994654e3625632066d20f43560ea656c5 Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 16:52:34 -0800 Subject: [PATCH 6/9] See OPS-177 : Fixing one more error --- lib/makara/proxy.rb | 70 ++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index 3f1ed905..1dd1d650 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -44,42 +44,6 @@ def send_to_all(*method_names) end end end - - private - - def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, method_name) - begin - should_retry = false - potentially_stale_connection = nil - - appropriate_connection(method_name, args) do |con| - con.send(method_name, *args, &block) - potentially_stale_connection = con - end - - rescue Exception => actual_exception - retry_attempts = retry_exceptions.inject({}) { |memo, retry_exception| memo[retry_exception['name']] = 0; memo } - retry_exceptions.each do |retry_exception| - if actual_exception.class.to_s == retry_exception['name'] - potentially_stale_connection.disconnect! - sleep retry_exception['time_between_retries_in_seconds'] || 0.1 - retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) - - if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS - should_retry = true - end - end - end - retry if should_retry - raise actual_exception - end - end - - def _execute_with_connection(args, block, method_name) - appropriate_connection(method_name, args) do |con| - con.send(method_name, *args, &block) - end - end end @@ -204,6 +168,40 @@ def any_connection end end + def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, method_name) + begin + should_retry = false + potentially_stale_connection = nil + + appropriate_connection(method_name, args) do |con| + con.send(method_name, *args, &block) + potentially_stale_connection = con + end + + rescue Exception => actual_exception + retry_attempts = retry_exceptions.inject({}) { |memo, retry_exception| memo[retry_exception['name']] = 0; memo } + retry_exceptions.each do |retry_exception| + if actual_exception.class.to_s == retry_exception['name'] + potentially_stale_connection.disconnect! + sleep retry_exception['time_between_retries_in_seconds'] || 0.1 + retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) + + if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS + should_retry = true + end + end + end + retry if should_retry + raise actual_exception + end + end + + def _execute_with_connection(args, block, method_name) + appropriate_connection(method_name, args) do |con| + con.send(method_name, *args, &block) + end + end + # based on the method_name and args, provide the appropriate connection # mark this proxy as hijacked so the underlying connection does not attempt to check # with back with this proxy. From 90bc73746a91875b1d421cac96f46c205d61fcbf Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 17:24:08 -0800 Subject: [PATCH 7/9] Fixing max-retry count error --- lib/makara/proxy.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index 1dd1d650..a80d8537 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -18,8 +18,6 @@ class Proxy < ::SimpleDelegator self.hijack_methods = [] class << self - MAX_RETRY_ATTEMPTS = 10 - def hijack_method(*method_names) self.hijack_methods = self.hijack_methods || [] self.hijack_methods |= method_names @@ -168,6 +166,8 @@ def any_connection end end + MAX_RETRY_COUNT = 10 + def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, method_name) begin should_retry = false @@ -186,7 +186,8 @@ def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, sleep retry_exception['time_between_retries_in_seconds'] || 0.1 retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) - if retry_attempt < retry_exception['retry_count'] && retry_attempt < MAX_RETRY_ATTEMPTS + max_attempts = retry_exception['retry_count'] || MAX_RETRY_COUNT + if retry_attempt < max_attempts should_retry = true end end From d43a219402d348a666c64ab9389a370089172489 Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Fri, 12 Nov 2021 17:25:30 -0800 Subject: [PATCH 8/9] See OPS-177 : Minor fix to the max number of attempts logic --- lib/makara/proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index a80d8537..69a03a74 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -187,7 +187,7 @@ def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1) max_attempts = retry_exception['retry_count'] || MAX_RETRY_COUNT - if retry_attempt < max_attempts + if retry_attempt < max_attempts && retry_attempt < MAX_RETRY_COUNT should_retry = true end end From 992ef926c2a08ad7fa88dd6c25dc8c11f69c1f05 Mon Sep 17 00:00:00 2001 From: Vishnu Narang Date: Mon, 15 Nov 2021 10:52:23 -0800 Subject: [PATCH 9/9] See OPS-177 : Minor fix to the retry logic --- lib/makara/proxy.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/makara/proxy.rb b/lib/makara/proxy.rb index 69a03a74..05e9af22 100644 --- a/lib/makara/proxy.rb +++ b/lib/makara/proxy.rb @@ -171,18 +171,15 @@ def any_connection def _execute_with_connection_and_retry_exceptions(retry_exceptions, args, block, method_name) begin should_retry = false - potentially_stale_connection = nil appropriate_connection(method_name, args) do |con| con.send(method_name, *args, &block) - potentially_stale_connection = con end rescue Exception => actual_exception retry_attempts = retry_exceptions.inject({}) { |memo, retry_exception| memo[retry_exception['name']] = 0; memo } retry_exceptions.each do |retry_exception| if actual_exception.class.to_s == retry_exception['name'] - potentially_stale_connection.disconnect! sleep retry_exception['time_between_retries_in_seconds'] || 0.1 retry_attempt = (retry_attempts[actual_exception.class.to_s] += 1)