Add a cop forbidding use of Kaminari.paginate_array
Description of the proposal
GitLab uses pagination (whether page-based or keyset) in its API and controller endpoints for performance reasons - a HTTP response containing every item of a collection simply doesn't scale well as the number of items increases, so we make the client request things a page (or a cursor, for graphql) at a time. This is great, and means a number of endpoints handle problems like "a project has 25,000 issues" really well.
However, pagination only has benefits where we push the pagination down, into the data store (be that the database, gitaly, redis, or something else entirely). I've looked at a number of issues today where we have performance issues in paginated endpoints because, instead of doing this, we load the entire dataset into Rails and then use Kaminari.paginate_array
to get a single page of data to return to the client.
A quick grep tells me there are 50 existing instances of Kaminari.paginate_array
in the GitLab codebase. Some may be acceptable, but many will be the cause of performance issues like the following:
So, I propose we add a cop that blacklists Kaminari.paginate_array
. It's a pretty clear signal that we're doing something wrong.
In cases where it is the right thing to do (e.g. this site in the elasticsearch integration: https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/gitlab/elastic/search_results.rb#L48), we can #rubocop:disable
it, but I think there's a lot of value in making people stop and think before taking a Ruby array and applying pagination to it.
-
Mention the proposal in the next backend weekly call and the #backend channel to encourage contribution -
Proceed with the proposal once 50% of the maintainers have weighed in, and 80% of the votes are 👍 -
Once approved, mention it again in the next backend weekly call and the #backend channel