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

Prevent deadlocks when updating managed service instances #2970

Conversation

philippthun
Copy link
Member

@philippthun philippthun commented Sep 19, 2022

Problem

A service instance was in state "update in progress" but the broker did not receive any request. Retrying the update was not possible (CF-AsyncServiceInstanceOperationInProgress).

Root cause

Updating the operation (see [1]) and creating the background job ([2]) is done independently. When the latter fails (e.g. database temporarily not available), the so called UpdaterLock ([3]) is not cleared.

Solution

Introduce similar methods as for ServiceBrokerUpdate ([4]):

  • update_broker_needed?
  • update_sync
  • enqueue_update.

Ensure that operation (lock) and delayed job are handled consistently, i.e. when the creation of the job fails, the operation will be set to "failed". This is achieved by moving the creation of the delayed job into the ServiceInstanceUpdateManaged action (method enqueue_update):

lock = UpdaterLock.new(...)
lock.lock!

begin
  update_job = UpdateServiceInstanceJob.new(...)
  pollable_job = Jobs::Enqueuer.new(update_job, ...).enqueue_pollable
  lock.asynchronous_unlock!
ensure
  lock.unlock_and_fail! if lock.present? && lock.needs_unlock?
end

[1]

service_instance, continue_async = action.try_update_sync

[2]
pollable_job = Jobs::Enqueuer.new(update_job, queue: Jobs::Queues.generic).enqueue_pollable

[3] https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/actions/services/locks/updater_lock.rb
[4] https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/actions/v3/service_broker_update.rb

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@philippthun philippthun force-pushed the prevent-deadlocks-when-updating-managed-service-instances branch from beb8b6a to 7763269 Compare September 20, 2022 07:33
@philippthun philippthun changed the title wip Prevent deadlocks when updating managed service instances Sep 20, 2022
@philippthun philippthun marked this pull request as ready for review September 20, 2022 07:42
Problem
-------
A service instance was in state "update in progress" but the broker did
not receive any request. Retrying the update was not possible
(CF-AsyncServiceInstanceOperationInProgress).

Root cause
----------
Updating the operation (see [1]) and creating the background job ([2])
is done independently. When the latter fails (e.g. database temporarily
not available), the so called UpdaterLock ([3]) is not cleared.

Solution
--------
Introduce similar methods as for ServiceBrokerUpdate ([4]):
- update_broker_needed?
- update_sync
- enqueue_update.
Ensure that operation (lock) and delayed job are handled consistently,
i.e. when the creation of the job fails, the operation will be set to
"failed". This is achieved by moving the creation of the delayed job
into the ServiceInstanceUpdateManaged action (method "enqueue_update"):

  lock = UpdaterLock.new(...)
  lock.lock!

  begin
    update_job = UpdateServiceInstanceJob.new(...)
    pollable_job = Jobs::Enqueuer.new(update_job, ...).enqueue_pollable
    lock.asynchronous_unlock!
  ensure
    lock.unlock_and_fail! if lock.present? && lock.needs_unlock?
  end

[1] https://github.com/cloudfoundry/cloud_controller_ng/blob/05f6cd8f8d3529d5bf8aaccfa2b9c4e75bbfb497/app/controllers/v3/service_instances_controller.rb#L292
[2] https://github.com/cloudfoundry/cloud_controller_ng/blob/05f6cd8f8d3529d5bf8aaccfa2b9c4e75bbfb497/app/controllers/v3/service_instances_controller.rb#L301
[3] https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/actions/services/locks/updater_lock.rb
[4] https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/actions/v3/service_broker_update.rb
@philippthun philippthun force-pushed the prevent-deadlocks-when-updating-managed-service-instances branch from 7763269 to 88bba63 Compare January 16, 2023 15:42
@philippthun philippthun requested a review from johha January 16, 2023 15:46
Copy link
Contributor

@johha johha left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@philippthun philippthun merged commit f70cb15 into cloudfoundry:main Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants