Skip to content

Change after_enqueue of enqueuer to before_enqueue and generate guid for delayed_job#1973

Merged
weymanf merged 2 commits intocloudfoundry:mainfrom
svkrieger:ISSUE-1950-fix-hanging-v3-delete-requests
Dec 9, 2020
Merged

Change after_enqueue of enqueuer to before_enqueue and generate guid for delayed_job#1973
weymanf merged 2 commits intocloudfoundry:mainfrom
svkrieger:ISSUE-1950-fix-hanging-v3-delete-requests

Conversation

@svkrieger
Copy link
Copy Markdown
Contributor

@svkrieger svkrieger commented Nov 24, 2020

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

  • A short explanation of the proposed change:
    The PR moves the logic for creating an entry in the ccdb's jobs table from the after_enqueue hook to the before_enqueue hook. With this it is guaranteed that the entry in the jobs table is already created before the job entry in the ccdb's delayed_jobs table is created and picked up by the worker.

  • An explanation of the use cases your change solves
    The PR is an suggestion for solving the ISSUE: Hanging cf api v3 DELETE requests (cf7 delete, delete-route, delete-space, unset-space-role)  #1950
    It is not ready for production, as the unit tests in spec/unit/jobs/pollable_job_wrapper_spec.rb fail due to the changes.
    It could have possible side-effects and require your review.

  • Links to any other associated PRs

  • I have reviewed the contributing guide

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

  • I have made this pull request to the master branch

  • I have run all the unit tests using bundle exec rake
    Yes but the tests in spec/unit/jobs/pollable_job_wrapper_spec.rb failed due to the logic changes.

  • I have run CF Acceptance Tests

@cf-gitbot
Copy link
Copy Markdown

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/175876867

The labels on this github issue will be updated when the story is started.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Nov 24, 2020

CLA Signed

The committers are authorized under a signed CLA.

@svkrieger svkrieger force-pushed the ISSUE-1950-fix-hanging-v3-delete-requests branch from 385ad5c to 8cdd06f Compare November 25, 2020 09:48
@svkrieger svkrieger force-pushed the ISSUE-1950-fix-hanging-v3-delete-requests branch from 8cdd06f to 7ec5204 Compare December 3, 2020 16:12
@cf-gitbot

This comment has been minimized.

@sweinstein22 sweinstein22 changed the base branch from master to main December 9, 2020 00:38
@weymanf weymanf merged commit 88959cc into cloudfoundry:main Dec 9, 2020
tpoland pushed a commit to tpoland/cloud_controller_ng that referenced this pull request Jan 5, 2021
Adding test to validate that the JobPollableModel insert happens
before the DelayedJob insert when an asynchronous task is enqueued.

This is done by adding callbacks that wrap the DelayedJob before
Callback and also execute before the after Callback to extract the
current state of each model as the callback lifecycle progresses.
This test is designed to validate the elimination of the race
condition identified in issue cloudfoundry#1973 and prevent reoccurrence.
tpoland pushed a commit to tpoland/cloud_controller_ng that referenced this pull request Jan 6, 2021
Adding test to validate that the JobPollableModel insert happens
before the DelayedJob insert when an asynchronous task is enqueued.

This is done by adding callbacks that wrap the DelayedJob before
Callback and also execute before the after Callback to extract the
current state of each model as the callback lifecycle progresses.
This test is designed to validate the elimination of the race
condition identified in issue cloudfoundry#1973 and prevent reoccurrence.
tpoland pushed a commit to tpoland/cloud_controller_ng that referenced this pull request Jan 6, 2021
Adding test to validate that the JobPollableModel insert happens
before the DelayedJob insert when an asynchronous task is enqueued.

This is done by adding callbacks that wrap the DelayedJob before
Callback and also execute before the after Callback to extract the
current state of each model as the callback lifecycle progresses.
This test is designed to validate the elimination of the race
condition identified in issue cloudfoundry#1973 and prevent reoccurrence.
tpoland pushed a commit to tpoland/cloud_controller_ng that referenced this pull request Jan 12, 2021
Adding test to validate that the JobPollableModel insert happens
before the DelayedJob insert when an asynchronous task is enqueued.

This is done by adding callbacks that wrap the DelayedJob before
Callback and also execute before the after Callback to extract the
current state of each model as the callback lifecycle progresses.
This test is designed to validate the elimination of the race
condition identified in issue cloudfoundry#1973 and prevent reoccurrence.
tpoland pushed a commit to tpoland/cloud_controller_ng that referenced this pull request Jan 13, 2021
Adding test to validate that the JobPollableModel insert happens
before the DelayedJob insert when an asynchronous task is enqueued.

This is done by adding callbacks that wrap the DelayedJob before
Callback and also execute before the after Callback to extract the
current state of each model as the callback lifecycle progresses.
This test is designed to validate the elimination of the race
condition identified in issue cloudfoundry#1973 and prevent reoccurrence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants