Blacklist the use of destroy_all
What does this MR do?
This merge request adds a RuboCop cop that blacklists the use of .destroy_all
, except for existing offences.
destroy_all
is similar to dependent: :destroy
, and blacklisted for the same reason: it loads every row into memory, then issues a DELETE
for every individual row. This in turn can be a disaster for both performance and availability. This lead to us blacklisting dependent: :destroy
in the past, and with this MR we're applying the same pattern to destroy_all
.
Sometimes a model may use a set of hooks to remove non DB data. Such logic should be moved outside of a model as this approach won't work for bulk removals. This is already documented in https://docs.gitlab.com/ee/development/foreign_keys.html#dependent-removals.
By adding this cop, database engineers (or other reviewers for that matter) no longer have to remind MR authors to avoid the use of destroy_all
.
EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6914
Does this MR meet the acceptance criteria?
-
Tests added for this feature/bug -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides