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

recover from RecordNotUnique race condition when creating new legacy appeal #15391

Conversation

tomas-nava
Copy link
Contributor

Description

If LegacyAppeal.find_or_create_by_vacols_id is hit multiple times for an appeal that doesn't yet exist, it's possible for a race condition to arise where the appeal is saved by one process in between the time another process initializes and attempts to save it.

I observed this happening when loading the case details page for appeals that exist in VACOLS but not Caseflow, when find_or_create_by_vacols_id is invoked from the appeals controller and the tasks controller in quick succession. When the second process attempts to save its version of the appeal, it fails with PG::UniqueViolation / ActiveRecord::RecordNotUnique.

I changed the method to capture the ActiveRecord::RecordNotUnique and return the existing record via find_by! (which'll raise a ActiveRecord::RecordNotFound if it's unsuccessful), following this pattern, from the Rails 6 create_or_find_by method.

It's not ideal to use an exception as flow control, but I think it's okay in this case to let the database constraint determine the action.

@codeclimate
Copy link

codeclimate bot commented Oct 6, 2020

Code Climate has analyzed commit 3e7f7a3 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Contributor

@ferristseng ferristseng left a comment

Choose a reason for hiding this comment

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

🎉 LGTM!

@tomas-nava tomas-nava added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Oct 6, 2020
@va-bot va-bot merged commit 5e83048 into master Oct 6, 2020
@va-bot va-bot deleted the tomas/recover-from-race-condition-when-loading-legacy-appeal-case-details branch October 6, 2020 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Eng: Backend Work Group backend engineering task Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Tango 💃
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants