Autosave changes disappearing and speed improvement #51065
Conversation
* main: (39 commits) Address `ActiveStorage::VariantTest#test_resized_variation_of_WEBP_blob` failure at Rails Nightly CI doc: Warn against having uniqueness validator when create_or_find_by is used [Fix rails#50604] Restore compatibility of ARE configs with eager loading mode ActionDispatch::Executor: report errors handled by ShowExceptions docker-entrypoint: export LD_PRELOAD Update create has_one failure test descriptions Update 7.0 release notes re inflections and the once autoloader Fix deprecated `enum` syntax in tests Deprecate defining enums with keywords args Fix configuration order Add note about imagemagick / libvips support being required Add WebP as a new framework default image type Add class name to enum validation exception message Update activestorage/app/models/active_storage/blob/representable.rb Transform actionpack documentation to Markdown docs:expand documentation on has_one_attached method. Disambiguate language around when code stops re: `redirect_to` Prefer to reference git's default configuration, not bespoke Fixes race condition when multiple preprocessed variants are defined for a Previewable file is attached Parse tests with prism ...
|
This solution solves both of those and still allow people to call save in I'm open to ideas on how to improve it, I assume you mean adding another module similar to |
|
It is hard to provide ideas on how to improve because this is reaching for too much. Passing this hash around for so many methods in Rails is a no go. I don't have right now another suggestion, but I just want to make it clear that a solution like this would never be merged. All that complexity added to There are many avenues to try here. Maybe there is an abstraction that is missing (see as example |
|
Actually. I'll close the PR since we will not merge this one. In the case you are interested on building a good solution for the problem, we can open new ones. I really appreciate you spending time trying to fix this issue, but this isn't a good solution. |
|
Leaving this gist here for anyone who has the same issue or think they might. You can just add this to an |
Motivation / Background
When
autosave: trueis set on both ends of a relationsaveand/orvalid?can be called more than once removing the changes that just happened during the currentsaveand makingprevious_changesand empty hash.Additionally when there is a lot of records in memory save on a sub record can be called a significant amount of times. It seem that the child models are saved and validated in every permutation possible. This PR results in every record being called at most 1 time and can significantly reduce CPU usage and time to save in this situations.
Detail
This PR adds
memoryto ensure that each record only gets saved and/or validated only once while traversing the autosave tree, ensuring the changes are available after saving via theprevious_changesmethod.Fixes #28491.
#28491 suggested adding a variable to avoid clearing the changes on the root record which worked; but would not fix the issue if the two models with the autosave true are related to another separate model that is being saved or updated.
Fixes #46438.
This PR also supplants #46438 and enhances it by being able to work across connections and increase speed by only allowing autosaved child models to be saved or validated only once.
Checklist
[Fix #issue-number]