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 500 on parallel object creation #3918

Merged
merged 6 commits into from
Aug 22, 2024

Conversation

kathap
Copy link
Contributor

@kathap kathap commented Aug 2, 2024

While fixing #3899, we found further possible occurrences of 500 errors in severel _create actions when doing parallel requests. In this PR we want to address them by adding an around_save to the corresponding model in which we catch Sequel::UniqueConstraintViolation errors for the case when ne POST bypasses the other one and inserts the record into the db after the first POST has passed the validate function but before the actual insert.

  • A short explanation of the proposed change:
    Adding an around_save to the corresponding model in which we catch Sequel::UniqueConstraintViolation errors for the case when ne POST bypasses the other one and inserts the record into the db after the first POST has passed the validate function but before the actual insert

  • An explanation of the use cases your change solves
    No more 500 errors on parallel request execution

  • Links to any other associated PRs
    Prevent 500 on parallel service instance creation #3899
    Prevent 500 on parallel org quota creation #3924

  • 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

@kathap kathap marked this pull request as draft August 2, 2024 11:45
@kathap kathap force-pushed the prevent-500-on-parallel-object-creation branch 2 times, most recently from 9c76c97 to b636892 Compare August 6, 2024 13:23
@kathap kathap marked this pull request as ready for review August 8, 2024 13:23
kathap added a commit that referenced this pull request Aug 8, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
While fixing #3899, we found further possible occurrences of 500 errors in
severel _create actions when doing parallel requests.
In this PR we want to address them by adding an around_save to the
corresponding model in which we catch Sequel::UniqueConstraintViolation
errors for the case when  ne POST bypasses the other one and inserts the record into the db after the first POST has passed the validate function but before the actual insert.
@kathap kathap force-pushed the prevent-500-on-parallel-object-creation branch from d5e3683 to fea4efb Compare August 8, 2024 13:36
Copy link
Contributor

@svkrieger svkrieger left a comment

Choose a reason for hiding this comment

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

Looks great! Just a question about mocking in two tests and a minor finding.

EDIT: Also do we plan to get rid of the unique checks in the coding? If we catch the unique constraint errors thrown by the DB reliably now, it should not be necessary to check for uniqueness beforehand.

spec/unit/isolation_segment_create_spec.rb Outdated Show resolved Hide resolved
@svkrieger svkrieger self-requested a review August 19, 2024 12:06
@kathap kathap marked this pull request as draft August 19, 2024 12:48
@kathap kathap marked this pull request as ready for review August 22, 2024 12:25
@kathap kathap merged commit 38855fd into main Aug 22, 2024
8 checks passed
ari-wg-gitbot added a commit to cloudfoundry/capi-release that referenced this pull request Aug 22, 2024
Changes in cloud_controller_ng:

- Prevent 500 on parallel object creation
    PR: cloudfoundry/cloud_controller_ng#3918
    Author: kathap <[email protected]>
@moleske moleske deleted the prevent-500-on-parallel-object-creation branch August 23, 2024 01:40
svkrieger pushed a commit that referenced this pull request Oct 16, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
svkrieger pushed a commit that referenced this pull request Oct 16, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
svkrieger pushed a commit that referenced this pull request Oct 17, 2024
To apply this change the migration from
#3923 needs to
be in first. Tha-s why we ceeated the adoption to prevetn 500 on
concurrent requests for organization_quota_create separately. The reason
is the same as in #3899 and #3918.
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.

2 participants