Skip to content

Document not using ActiveRecord's serialize method

yorickpeterse-staging requested to merge document-not-using-serialize into master

This MR makes two changes:

  1. It adds documentation stating that one should avoid storing serialized data in the database unless truly necessary (and that these are very rare cases)
  2. It adds a Rubocop Cop that enforces this, ignoring existing instances so we don't immediately have 27 offences

These changes are based on past problems we have had with serialized data (e.g. events storing massive YAML blobs of which only a tiny fraction is used) or cases where developers are considering adding serialized data (e.g. https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1976/).

To summarise why serialised data is a bad idea:

  • 99% of the time you just really don't need it
  • It's a total pain to migrate
  • You can't query by the data
  • You can't index the data properly
  • It's a waste of storage space
  • It's a waste of CPU time as every time you retrieve the data you have to parse it, this in turn could affect page loading times
  • It's a waste of RAM as parsing the data structures will require memory for any ASTs and such
  • We're using a relational database, not a document store such as MongoDB

cc @smcgivern @dzaporozhets @DouweM @rspeicher

Merge request reports