- Notifications
You must be signed in to change notification settings - Fork22.1k
Offer dependent: :destroy_async for associations#40157
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Offer dependent: :destroy_async for associations#40157
Uh oh!
There was an error while loading.Please reload this page.
Conversation
0622f67 to26ce831Compareaee646b to66c4747CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
ee96146 toc112d8eComparec112d8e tof6d41ecCompareSometimes cascading association deletions can cause timeouts due toan IO issue. Perhaps a model has associations that are destroyed ondeletion which in turn trigger other deletions and this can continuedown a complex tree. Along this tree you may also hit other IOoperations. Such deep deletions can lead to server timeouts whileawaiting completion and really the user may not notice all thechanges on their side immediately making them wait unnecesarially orworse causing a timeout during the operation.We now allow associations supporting the `dependent:` key to take `:destroy_async`,which schedules a background job to destroy associations.Co-authored-by: Adrianna Chang <adrianna.chang@shopify.com>Co-authored-by: Rafael Mendonça França <rafael@franca.dev>Co-authored-by: Cory Gwin@gwincr11 <gwincr11@github.com>
f6d41ec to4cf7559Comparegwincr11 commentedSep 25, 2020
kylefox commentedOct 5, 2020
This is awesome 👍 But just curious: why not use That seems more consistent with other delayed methods: Is there something about the behavior that makes |
| valid=super +[:counter_cache,:join_table,:index_errors,:ensuring_owner_was] | ||
| valid +=[:as,:foreign_type]ifoptions[:as] | ||
| valid +=[:through,:source,:source_type]ifoptions[:through] | ||
| valid +=[:ensuring_owner_was]ifoptions[:dependent] ==:destroy_async |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
isn't it a duplicate with the addition 3 lines above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, good catch@vendethiel ! Feel free to open a PR ❤️
| defself.valid_options(options) | ||
| valid=super +[:counter_cache,:join_table,:index_errors] | ||
| valid=super +[:counter_cache,:join_table,:index_errors,:ensuring_owner_was] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As@vendethiel said, here the key is always included, which invalidates line 13, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As@adrianna-chang-shopify said, feel free to open a PR 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Here@eugeneius , could you review?@adrianna-chang-shopify
kylefox commentedNov 19, 2020
@rafaelfranca@adrianna-chang-shopify I don't want to be a pest 😅 but I think it would be useful for the community to understand why this option is named |
rafaelfranca commentedNov 19, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The reason for the different name if because we wanted to keep the For the feature implemented here we don't care if the destroy happen in 1s or in 10 minutes. It just need to happen async, without blocking the current request. So the behavior of |
Motivation: - Now that destroy_async is an option on association, a natural next step seems to be adding this method to standard objects for a more consistent offering.Related Issues: -rails#40157 -rails#36912Changes: - Add a destroy_async method to persistence. - Add a rail tie to setup the active job configs. - Add a default destroy async job. - Test
tomrossi7 commentedAug 4, 2023
Would a PR to add |
jarthod commentedOct 31, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@tomrossi7 👍 on And also thanks for this |

Uh oh!
There was an error while loading.Please reload this page.
Revised version of#36912,#39149
Summary
Allows associations taking the
dependentkey to specifydependent: :destroy_async, which will enqueue a job to destroy associations asynchronously.cc@rafaelfranca - I've removed
ActiveRecord::Base.destroy_laterfrom this PR, so it only contains the implementation of the asynchronous association deletion. Will rebase + squash if it looks okay to you.