Switch to cursor-based pagination with geared_pagination #363
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #362
As mentioned in the issue, Pagy uses offset-based pagination, which decreases in performance as the collection of elements being paginated grows. It doesn't scale very well, and queries that use
OFFSET
are outright banned in Core for performance reasons. A cursor based pagination strategy queries based on a specific point in the dataset (the cursor), and works well forsequential datasets / datasets with ordered attributes (
maintenance_tasks_runs
table is guaranteed to be sortable bycreated_at
).Basecamp has actually written a gem that supports cursor-based pagination, which I think is a really good candidate for us to use, replacing Pagy.
Implementation
set_page_and_extract_portion_from
with theordered_by
option to perform cursor-based pagination#previous_runs
, since the pagination will handle this for us now@page
, which contains the recordset of paginated elements which we can render in the view (similar to what was happening with Pagy)GearedPagination::Controller
inActionController::Base
when the gem is initialized-
GearedPagination::Controller
sets anafter_action
filter,set_paginated_headers
- This currently breaks our test suite, because
set_paginated_headers
checks an ivar that doesn't exist (@page
) which warns and thus raises in our test suite. I also don't really like that this filter runs before every single controller action in the engine and the host app.- So, I've added an initializer that removes this filter and puts the onus on us to call
set_paginated_headers
when we need to (for now, this is only inTasksController#show
)- Arguably this is a little brittle. We could write a wrapper to isolate the call to
set_paginated_headers
in case we want to reuse pagination elsewhere, but I thought maybe that was prematureTo 🎩
runs
in the dummy (ie.rails db:seed
a couple times)Tradeoffs