From e6f2c5ddcd8b8faba1f27cd9890fe8f24e6b72c8 Mon Sep 17 00:00:00 2001 From: Lucas Di Pentima Date: Tue, 21 Jul 2020 14:21:49 -0300 Subject: [PATCH] 16470: Adds an explicit reload before every pending with_lock call. Previous versions of rails did an implicit reload when calling with_lock, but from 5.2 calling with_lock with unpersisted changes will raise an exception. Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima --- .../controllers/arvados/v1/containers_controller.rb | 2 +- services/api/app/models/container.rb | 10 +++++----- services/api/app/models/job.rb | 2 +- services/api/test/unit/collection_test.rb | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb index 041f559472..b2324a5712 100644 --- a/services/api/app/controllers/arvados/v1/containers_controller.rb +++ b/services/api/app/controllers/arvados/v1/containers_controller.rb @@ -29,7 +29,7 @@ class Arvados::V1::ContainersController < ApplicationController end def update - @object.with_lock do + @object.reload.with_lock do super end end diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb index adfbf6042f..16f8cd798f 100644 --- a/services/api/app/models/container.rb +++ b/services/api/app/models/container.rb @@ -339,7 +339,7 @@ class Container < ArvadosModel end def lock - self.with_lock do + self.reload.with_lock do if self.state != Queued raise LockFailedError.new("cannot lock when #{self.state}") end @@ -357,7 +357,7 @@ class Container < ArvadosModel end def unlock - self.with_lock do + self.reload.with_lock do if self.state != Locked raise InvalidStateTransitionError.new("cannot unlock when #{self.state}") end @@ -654,7 +654,7 @@ class Container < ArvadosModel # This container is finished so finalize any associated container requests # that are associated with this container. if saved_change_to_state? and self.final? - # These get wiped out by with_lock (which reloads the record), + # These get wiped out (with_lock requires to explicitly reload the record), # so record them now in case we need to schedule a retry. prev_secret_mounts = secret_mounts_before_last_save prev_runtime_token = runtime_token_before_last_save @@ -665,7 +665,7 @@ class Container < ArvadosModel # transaction finishes. This ensure that concurrent container # requests that try to reuse this container are finalized (on # Complete) or don't reuse it (on Cancelled). - self.with_lock do + self.reload.with_lock do act_as_system_user do if self.state == Cancelled retryable_requests = ContainerRequest.where("container_uuid = ? and priority > 0 and state = 'Committed' and container_count < container_count_max", uuid) @@ -690,7 +690,7 @@ class Container < ArvadosModel } c = Container.create! c_attrs retryable_requests.each do |cr| - cr.with_lock do + cr.reload.with_lock do leave_modified_by_user_alone do # Use row locking because this increments container_count cr.container_uuid = c.uuid diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb index 37e5f455df..31179f0666 100644 --- a/services/api/app/models/job.rb +++ b/services/api/app/models/job.rb @@ -136,7 +136,7 @@ class Job < ArvadosModel end def lock locked_by_uuid - with_lock do + reload.with_lock do unless self.state == Queued and self.is_locked_by_uuid.nil? raise AlreadyLockedError end diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb index addea83062..dbfdae95ac 100644 --- a/services/api/test/unit/collection_test.rb +++ b/services/api/test/unit/collection_test.rb @@ -515,7 +515,7 @@ class CollectionTest < ActiveSupport::TestCase c2.description = 'foo collection' c1.save! assert_equal 1, c2.version - # with_lock forces a reload, so this shouldn't produce an unique violation error + # with_lock requires a reload, so this shouldn't produce an unique violation error c2.save! assert_equal 3, c2.version assert_equal 'foo collection', c2.description -- 2.30.2