-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Pagy in favour of cursor-based pagination #371
Conversation
@etiennebarrie could use your 👀 on this please! |
71e15d3
to
7d13c0f
Compare
@etiennebarrie I've tightened up the queries on this. Now they look something like this: Processing by MaintenanceTasks::TasksController#show as HTML
Parameters: {"cursor"=>"16", "id"=>"Maintenance::UpdatePostsTask"}
MaintenanceTasks::Run Load (0.2ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["LIMIT", 1]]
ActiveStorage::Attachment Load (0.1ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" = ? [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], ["record_id", 90]]
MaintenanceTasks::Run Exists? (0.2ms) SELECT 1 AS one FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? AND (id < '16') LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 1]]
MaintenanceTasks::Run Load (0.3ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? AND (id < '16') ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 20]]
ActiveStorage::Attachment Load (0.2ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" IN
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], [nil, 15], [nil, 14], [nil, 13], [nil, 12], [nil, 11], [nil, 10], [nil, 9], [nil, 8], [nil, 7], [nil, 6], [nil, 5], [nil, 4], [nil, 3], [nil, 2], [nil, 1]]
(0.2ms) SELECT "maintenance_tasks_runs"."id" FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? ORDER BY "maintenance_tasks_runs"."created_at" DESC [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90]] Compared to before, which was: Processing by MaintenanceTasks::TasksController#show as HTML
Parameters: {"cursor"=>"16", "id"=>"Maintenance::UpdatePostsTask"}
MaintenanceTasks::Run Load (0.2ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["LIMIT", 1]]
ActiveStorage::Attachment Load (0.1ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" = ? [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], ["record_id", 90]]
MaintenanceTasks::Run Exists? (0.2ms) SELECT 1 AS one FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? AND (id < '16') LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 1]]
MaintenanceTasks::Run Load (0.3ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? AND (id < '16') ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 20]]
ActiveStorage::Attachment Load (0.2ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" IN
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], [nil, 15], [nil, 14], [nil, 13], [nil, 12], [nil, 11], [nil, 10], [nil, 9], [nil, 8], [nil, 7], [nil, 6], [nil, 5], [nil, 4], [nil, 3], [nil, 2], [nil, 1]]
(0.2ms) MaintenanceTasks::Run Load (0.2ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? ORDER BY "maintenance_tasks_runs"."created_at" ASC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 1]]
ActiveStorage::Attachment Load (0.1ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" = ? [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], ["record_id", 1]]
CACHE MaintenanceTasks::Run Load (0.0ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? AND (id < '16') ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 20]]
CACHE ActiveStorage::Attachment Load (0.0ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" IN
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], [nil, 15], [nil, 14], [nil, 13], [nil, 12], [nil, 11], [nil, 10], [nil, 9], [nil, 8], [nil, 7], [nil, 6], [nil, 5], [nil, 4], [nil, 3], [nil, 2], [nil, 1]] This was achieved by:
|
There's still an
which calls We could use |
7d13c0f
to
fb27cfc
Compare
Thanks, forgot we could avoid the Processing by MaintenanceTasks::TasksController#show as HTML
Parameters: {"cursor"=>"16", "id"=>"Maintenance::UpdatePostsTask"}
MaintenanceTasks::Run Load (0.2ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["LIMIT", 1]]
ActiveStorage::Attachment Load (0.1ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" = ? [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], ["record_id", 90]]
MaintenanceTasks::Run Load (0.3ms) SELECT "maintenance_tasks_runs".* FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? AND (id < '16') ORDER BY "maintenance_tasks_runs"."created_at" DESC LIMIT ? [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90], ["LIMIT", 20]]
ActiveStorage::Attachment Load (0.2ms) SELECT "active_storage_attachments".* FROM "active_storage_attachments" WHERE "active_storage_attachments"."record_type" = ? AND "active_storage_attachments"."name" = ? AND "active_storage_attachments"."record_id" IN
(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) [["record_type", "MaintenanceTasks::Run"], ["name", "csv_file"], [nil, 15], [nil, 14], [nil, 13], [nil, 12], [nil, 11], [nil, 10], [nil, 9], [nil, 8], [nil, 7], [nil, 6], [nil, 5], [nil, 4], [nil, 3], [nil, 2], [nil, 1]]
(0.2ms) SELECT "maintenance_tasks_runs"."id" FROM "maintenance_tasks_runs" WHERE "maintenance_tasks_runs"."task_name" = ? AND "maintenance_tasks_runs"."id" != ? ORDER BY "maintenance_tasks_runs"."created_at" DESC [["task_name", "Maintenance::UpdatePostsTask"], ["id", 90]] |
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 for
sequential datasets / datasets with ordered attributes (
maintenance_tasks_runs
table is guaranteed to be sortable bycreated_at
/id
).This PR introduces a
RunsPage
model that handles generating a "page" ofRuns
based on a cursor (id
). Instead of callingTaskData#previous_runs
and passing this topagy
to be paginated, theTasksController#show
now initializes a newRunsPage
with this relation, and passes a cursor if it receives one from the params.RunsPage
has three methods -#records
is the runs to be rendered on the current page,#next_cursor
fetches the cursor to be used in order to move to the next page, andlast?
is used by the views to prevent showing theNext page
link if we're on the last page.I opted to keep these in a separate model instead of in
TaskData
because it was feeling like a clump of related data / behaviour and I wanted to be able to store the relation / cursor in ivars instead of having to pass them around as params inTaskData
methods.I also added a system test for pagination, but maybe we don't need this, let me know.