From 5d107fe5eef2946f3896ab3976c13747b276c414 Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Fri, 19 Feb 2021 17:46:02 -0500 Subject: [PATCH] Change cursor from bigint to string Working on the upcoming support for tasks backed by custom enumerators surfaced the need for non-numeric cursors. For instance, one might have a task that iterates over resources backed by some API providing its own opaque string cursor, which would be impossible to serialize to a bigint column. As such, this changes the maintenance_tasks_runs.cursor column to be a string. For this to work, we must deserialize the cursor back to an integer in the Array and CSV collection cases, while the ActiveRecord case is handled transparently. Co-authored-by: Adrianna Chang Co-authored-by: Sam Bostock --- .../maintenance_tasks/task_job_concern.rb | 6 +++--- .../20210219212931_change_cursor_to_string.rb | 20 +++++++++++++++++++ test/dummy/db/schema.rb | 2 +- test/jobs/maintenance_tasks/task_job_test.rb | 4 ++-- 4 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20210219212931_change_cursor_to_string.rb diff --git a/app/jobs/concerns/maintenance_tasks/task_job_concern.rb b/app/jobs/concerns/maintenance_tasks/task_job_concern.rb index cd55e16e..41108172 100644 --- a/app/jobs/concerns/maintenance_tasks/task_job_concern.rb +++ b/app/jobs/concerns/maintenance_tasks/task_job_concern.rb @@ -56,14 +56,14 @@ def build_enumerator(_run, cursor:) batch_size: collection.batch_size, ) when Array - enumerator_builder.build_array_enumerator(collection, cursor: cursor) + enumerator_builder.build_array_enumerator(collection, cursor: cursor&.to_i) when BatchCsvCollectionBuilder::BatchCsv JobIteration::CsvEnumerator.new(collection.csv).batches( batch_size: collection.batch_size, - cursor: cursor, + cursor: cursor&.to_i, ) when CSV - JobIteration::CsvEnumerator.new(collection).rows(cursor: cursor) + JobIteration::CsvEnumerator.new(collection).rows(cursor: cursor&.to_i) else raise ArgumentError, <<~MSG.squish #{@task.class.name}#collection must be either an diff --git a/db/migrate/20210219212931_change_cursor_to_string.rb b/db/migrate/20210219212931_change_cursor_to_string.rb new file mode 100644 index 00000000..eccd9ac8 --- /dev/null +++ b/db/migrate/20210219212931_change_cursor_to_string.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class ChangeCursorToString < ActiveRecord::Migration[6.0] + # This migration will clear all existing data in the cursor column for + # both MySQL and PostgresQL. Ensure no Tasks are paused when this migration + # is deployed, or they will be resumed from the start. + # Running tasks are able to gracefully handle this change, even if + # interrupted. + def up + change_table(:maintenance_tasks_runs) do |t| + t.change(:cursor, :string) + end + end + + def down + change_table(:maintenance_tasks_runs) do |t| + t.change(:cursor, :bigint) + end + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index ad8d7ad3..f686ae80 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -47,7 +47,7 @@ t.bigint "tick_count", default: 0, null: false t.bigint "tick_total" t.string "job_id" - t.bigint "cursor" + t.string "cursor" t.string "status", default: "enqueued", null: false t.string "error_class" t.string "error_message" diff --git a/test/jobs/maintenance_tasks/task_job_test.rb b/test/jobs/maintenance_tasks/task_job_test.rb index 4c1199b6..fc7ce560 100644 --- a/test/jobs/maintenance_tasks/task_job_test.rb +++ b/test/jobs/maintenance_tasks/task_job_test.rb @@ -233,11 +233,11 @@ class << self TaskJob.perform_now(@run) - assert_equal 0, @run.reload.cursor + assert_equal "0", @run.reload.cursor end test ".perform_now starts job from cursor position when job resumes" do - @run.update!(cursor: 0) + @run.update!(cursor: "0") Maintenance::TestTask.any_instance.expects(:process).once.with(2)