Added performance guidelines for new MRs
What does this MR do?
This MR adds a set of guides that should be followed by merge request authors.
Are there points in the code the reviewer needs to double check?
Spelling, grammar, etc
Why was this MR needed?
There is no set of guidelines one should follow when submitting merge requests. This leads to developers at times disregarding performance. This in turn results in performance specialists having to clean up the mess, or production engineers being woken up in the middle of the night because the database is on fire.
Does this MR meet the acceptance criteria?
-
Documentation created/updated - Tests
-
All builds are passing
-
-
Conform by the style guides -
Branch has no merge conflicts with master(if you do - rebase it please) -
Squashed related commits together
Merge request reports
Activity
Marked the task Conform by the style guides as completed
Marked the task Conform by the style guides as incomplete
Marked the task Conform by the style guides as completed
cc @jschatz1 since there's a small section on UI elements that we may want to expand/adjust.
Added 1 commit:
- 02354d39 - Added performance guidelines for new MRs
@pcarranza @rspeicher The guide has been adjusted based on your feedback.
Added 1 commit:
- e4e03d94 - Added performance guidelines for new MRs
Milestone changed to %8.12
@axil @pcarranza @rspeicher Any further feedback on these changes?
@yorickpeterse
@yorickpeterse awesome!
@axil Any comments? Can this be merged?
mentioned in commit ee61c403
61 ```ruby 62 objects_to_update.each do |object| 63 object.some_field = some_value 64 object.save 65 end 66 ``` 67 68 This will end up running one query for every object to update. This code can 69 easily overload a database given enough rows to update or many instances of this 70 code running in parallel. This particular problem is known as the 71 ["N+1 query problem"](http://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations). 72 73 In this particular case the workaround is fairly easy: 74 75 ```ruby 76 objects_to_update.update_all(some_field: some_value) We should probably add a comment that this will not execute
ARcallbacks:validations / (before|after)_(save|update). So, developer should ensure that object he is updating doesn't have ones defined or do pass validations as it can introduce hard to debug problems or database inconsistency.Edited by Kamil Trzcińśki@ayufan I'm not sure if I want to cover what's already documented in the Rails documentation. Having said that, we can add it if really necessary.
