From 4f4ee14d1986213076e204605863dd8f51004ff9 Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Thu, 6 Jul 2023 12:28:02 -0400 Subject: [PATCH 01/10] APPEALS-24987 Added redis-mutex to gemfile --- Gemfile | 1 + Gemfile.lock | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/Gemfile b/Gemfile index d8fedbfa5f9..6309ec0efa8 100644 --- a/Gemfile +++ b/Gemfile @@ -62,6 +62,7 @@ gem "rainbow" gem "react_on_rails", "11.3.0" gem "redis-namespace" gem "redis-rails", "~> 5.0.2" +gem 'redis-mutex' gem "request_store" gem "roo", "~> 2.7" # Use SCSS for stylesheets diff --git a/Gemfile.lock b/Gemfile.lock index 98ffb5dd195..31532c5bf26 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -523,6 +523,10 @@ GEM redis-activesupport (5.0.4) activesupport (>= 3, < 6) redis-store (>= 1.3, < 2) + redis-classy (2.4.1) + redis-namespace (~> 1.0) + redis-mutex (4.0.2) + redis-classy (~> 2.0) redis-namespace (1.6.0) redis (>= 3.0.4) redis-rack (2.0.4) @@ -783,6 +787,7 @@ DEPENDENCIES rainbow rb-readline react_on_rails (= 11.3.0) + redis-mutex redis-namespace redis-rails (~> 5.0.2) request_store From dee6c272886648e11ada5fab982cc7165a7233db Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Fri, 7 Jul 2023 12:29:08 -0400 Subject: [PATCH 02/10] APPEALS-24987 added RedisMutex init and lock --- app/models/end_product_establishment.rb | 5 +++++ config/initializers/redis_mutex.rb | 1 + 2 files changed, 6 insertions(+) create mode 100644 config/initializers/redis_mutex.rb diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index d75a0ca5254..f4e7934dc68 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -9,6 +9,9 @@ # the current status of the EP when the EndProductEstablishment is synced. class EndProductEstablishment < CaseflowRecord + + include RedisMutex::Macro + belongs_to :source, polymorphic: true belongs_to :user has_many :request_issues @@ -18,6 +21,8 @@ class EndProductEstablishment < CaseflowRecord has_one :priority_end_product_sync_queue belongs_to :vbms_ext_claim, foreign_key: "reference_id", primary_key: "claim_id", optional: true + auto_mutex :sync!, on: [:id] + # allow @veteran to be assigned to save upstream calls attr_writer :veteran diff --git a/config/initializers/redis_mutex.rb b/config/initializers/redis_mutex.rb new file mode 100644 index 00000000000..2bc65f75825 --- /dev/null +++ b/config/initializers/redis_mutex.rb @@ -0,0 +1 @@ +RedisClassy.redis = Redis.new(url: Rails.application.secrets.redis_url_cache) From 2c26bd789ab774ce1e8b38ceef365fc6b738ba4a Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Fri, 7 Jul 2023 13:03:43 -0400 Subject: [PATCH 03/10] removed "on" lock in EPE --- app/models/end_product_establishment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index f4e7934dc68..4cb9ff2ef77 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -21,7 +21,7 @@ class EndProductEstablishment < CaseflowRecord has_one :priority_end_product_sync_queue belongs_to :vbms_ext_claim, foreign_key: "reference_id", primary_key: "claim_id", optional: true - auto_mutex :sync!, on: [:id] + auto_mutex :sync! # allow @veteran to be assigned to save upstream calls attr_writer :veteran From 0deb1b25b4d923093d1106d0a747b97243bc2d75 Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Mon, 10 Jul 2023 12:37:43 -0400 Subject: [PATCH 04/10] added comments, retrying lock using "id" --- app/models/end_product_establishment.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index 4cb9ff2ef77..21295e9ffb6 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -9,7 +9,7 @@ # the current status of the EP when the EndProductEstablishment is synced. class EndProductEstablishment < CaseflowRecord - + # Using macro-style definition. The locking scope will be TheClass#method and only one method can run at any given time. include RedisMutex::Macro belongs_to :source, polymorphic: true @@ -21,7 +21,13 @@ class EndProductEstablishment < CaseflowRecord has_one :priority_end_product_sync_queue belongs_to :vbms_ext_claim, foreign_key: "reference_id", primary_key: "claim_id", optional: true - auto_mutex :sync! + # :block => 1 # Specify in seconds how long you want to wait for the lock to be released. + # # Specify 0 if you need non-blocking sematics and return false immediately. (default: 1) + # :sleep => 0.1 # Specify in seconds how long the polling interval should be when :block is given. + # # It is NOT recommended to go below 0.01. (default: 0.1) + # :expire => 10 # Specify in seconds when the lock should be considered stale when something went wrong + # # with the one who held the lock and failed to unlock. (default: 10) + auto_mutex :sync, block: 7, after_failure: lambda { render text: 'failed to acquire lock!' }, on: [id] # allow @veteran to be assigned to save upstream calls attr_writer :veteran @@ -217,6 +223,7 @@ def sync! contentions unless result.status_type_code == EndProduct::STATUSES.key("Canceled") transaction do + sleep 30 update!( synced_status: result.status_type_code, last_synced_at: Time.zone.now From a0839e82b36072e6479f7a329dae19070e4e8b13 Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Mon, 10 Jul 2023 12:45:31 -0400 Subject: [PATCH 05/10] removing lock on "id" --- app/models/end_product_establishment.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index 21295e9ffb6..d4b36bdca40 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -27,7 +27,7 @@ class EndProductEstablishment < CaseflowRecord # # It is NOT recommended to go below 0.01. (default: 0.1) # :expire => 10 # Specify in seconds when the lock should be considered stale when something went wrong # # with the one who held the lock and failed to unlock. (default: 10) - auto_mutex :sync, block: 7, after_failure: lambda { render text: 'failed to acquire lock!' }, on: [id] + auto_mutex :sync, block: 7, after_failure: lambda { render text: 'failed to acquire lock!' } # allow @veteran to be assigned to save upstream calls attr_writer :veteran @@ -47,6 +47,8 @@ class InvalidEndProductError < StandardError; end class ContentionNotFound < StandardError; end class << self + auto_mutex :sync, block: 7, after_failure: lambda { render text: 'failed to acquire lock!' }, on: [self.id] + def order_by_sync_priority active.order("last_synced_at IS NOT NULL, last_synced_at ASC") end From 0a39ae9776148b9628f60dddeaf5bb5ac275c5b6 Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Mon, 10 Jul 2023 12:52:49 -0400 Subject: [PATCH 06/10] deleting extra line --- app/models/end_product_establishment.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index d4b36bdca40..a5f40ab0cb5 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -47,8 +47,6 @@ class InvalidEndProductError < StandardError; end class ContentionNotFound < StandardError; end class << self - auto_mutex :sync, block: 7, after_failure: lambda { render text: 'failed to acquire lock!' }, on: [self.id] - def order_by_sync_priority active.order("last_synced_at IS NOT NULL, last_synced_at ASC") end From 07410ac88f3e691032171f88cc5d85f0358a78fc Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Wed, 12 Jul 2023 16:39:56 -0400 Subject: [PATCH 07/10] saving progress --- app/models/end_product_establishment.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index a5f40ab0cb5..ab9cb512fd4 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -27,7 +27,7 @@ class EndProductEstablishment < CaseflowRecord # # It is NOT recommended to go below 0.01. (default: 0.1) # :expire => 10 # Specify in seconds when the lock should be considered stale when something went wrong # # with the one who held the lock and failed to unlock. (default: 10) - auto_mutex :sync, block: 7, after_failure: lambda { render text: 'failed to acquire lock!' } + auto_mutex :sync!, block: 60, expire: 100, after_failure: lambda { Rails.logger.error('failed to acquire lock! EPE sync is being called by another process. Please try again later.') } # allow @veteran to be assigned to save upstream calls attr_writer :veteran @@ -211,6 +211,9 @@ def cancel_unused_end_product! end def sync! + sleep(1) + puts "entered block" + byebug # There is no need to sync end_product_status if the status # is already inactive since an EP can never leave that state return true unless status_active? @@ -223,7 +226,6 @@ def sync! contentions unless result.status_type_code == EndProduct::STATUSES.key("Canceled") transaction do - sleep 30 update!( synced_status: result.status_type_code, last_synced_at: Time.zone.now From 4280c5e9e0f2ebfe7570f2b674c35d746448d66c Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Thu, 13 Jul 2023 10:09:09 -0400 Subject: [PATCH 08/10] removing debugging lines --- app/models/end_product_establishment.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index ab9cb512fd4..e776e63052f 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -212,8 +212,6 @@ def cancel_unused_end_product! def sync! sleep(1) - puts "entered block" - byebug # There is no need to sync end_product_status if the status # is already inactive since an EP can never leave that state return true unless status_active? From 6b59bbbb6e96a27af4251a71caf82eff2e01243e Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Fri, 14 Jul 2023 10:50:56 -0400 Subject: [PATCH 09/10] changed mutex logic to use "with_lock" --- app/models/end_product_establishment.rb | 54 +++++++++++-------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index e776e63052f..aa4574875c4 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -9,9 +9,6 @@ # the current status of the EP when the EndProductEstablishment is synced. class EndProductEstablishment < CaseflowRecord - # Using macro-style definition. The locking scope will be TheClass#method and only one method can run at any given time. - include RedisMutex::Macro - belongs_to :source, polymorphic: true belongs_to :user has_many :request_issues @@ -21,14 +18,6 @@ class EndProductEstablishment < CaseflowRecord has_one :priority_end_product_sync_queue belongs_to :vbms_ext_claim, foreign_key: "reference_id", primary_key: "claim_id", optional: true - # :block => 1 # Specify in seconds how long you want to wait for the lock to be released. - # # Specify 0 if you need non-blocking sematics and return false immediately. (default: 1) - # :sleep => 0.1 # Specify in seconds how long the polling interval should be when :block is given. - # # It is NOT recommended to go below 0.01. (default: 0.1) - # :expire => 10 # Specify in seconds when the lock should be considered stale when something went wrong - # # with the one who held the lock and failed to unlock. (default: 10) - auto_mutex :sync!, block: 60, expire: 100, after_failure: lambda { Rails.logger.error('failed to acquire lock! EPE sync is being called by another process. Please try again later.') } - # allow @veteran to be assigned to save upstream calls attr_writer :veteran @@ -211,28 +200,31 @@ def cancel_unused_end_product! end def sync! - sleep(1) - # There is no need to sync end_product_status if the status - # is already inactive since an EP can never leave that state - return true unless status_active? - - fail EstablishedEndProductNotFound, id unless result - - # load contentions now, in case "source" needs them. - # this VBMS call is slow and will cause the transaction below - # to timeout in some cases. - contentions unless result.status_type_code == EndProduct::STATUSES.key("Canceled") + RedisMutex.with_lock(self.id.to_s, block: 60, expire: 100) do # key => "EndProductEstablishment:id" + # There is no need to sync end_product_status if the status + # is already inactive since an EP can never leave that state + return true unless status_active? + + fail EstablishedEndProductNotFound, id unless result + + # load contentions now, in case "source" needs them. + # this VBMS call is slow and will cause the transaction below + # to timeout in some cases. + contentions unless result.status_type_code == EndProduct::STATUSES.key("Canceled") + + transaction do + update!( + synced_status: result.status_type_code, + last_synced_at: Time.zone.now + ) + status_cancelled? ? handle_cancelled_ep! : sync_source! + close_request_issues_with_no_decision! + end - transaction do - update!( - synced_status: result.status_type_code, - last_synced_at: Time.zone.now - ) - status_cancelled? ? handle_cancelled_ep! : sync_source! - close_request_issues_with_no_decision! + save_updated_end_product_code! end - - save_updated_end_product_code! + rescue RedisMutex::LockError + Rails.logger.error('failed to acquire lock! EPE sync is being called by another process. Please try again later.') rescue EstablishedEndProductNotFound, AppealRepository::AppealNotValidToReopen => error raise error rescue StandardError => error From f60306257babae8b31b2e12b9288e43f8b41ff9a Mon Sep 17 00:00:00 2001 From: Jonathan Tsang Date: Fri, 14 Jul 2023 13:49:28 -0400 Subject: [PATCH 10/10] updated mutex lock key --- app/models/end_product_establishment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/end_product_establishment.rb b/app/models/end_product_establishment.rb index aa4574875c4..8c637f6a09e 100644 --- a/app/models/end_product_establishment.rb +++ b/app/models/end_product_establishment.rb @@ -200,7 +200,7 @@ def cancel_unused_end_product! end def sync! - RedisMutex.with_lock(self.id.to_s, block: 60, expire: 100) do # key => "EndProductEstablishment:id" + RedisMutex.with_lock("EndProductEstablishment:#{id}", block: 60, expire: 100) do # key => "EndProductEstablishment:id" # There is no need to sync end_product_status if the status # is already inactive since an EP can never leave that state return true unless status_active?