From 984524fbc3c04c04337137672554c25c8ea6c0de Mon Sep 17 00:00:00 2001 From: Andrew Cain Date: Fri, 5 Jan 2024 21:03:11 +1100 Subject: [PATCH] fix: change tii batch upload to limit submission rate --- app/models/turn_it_in/tii_action.rb | 16 ++++++++++------ app/models/turn_it_in/tii_action_fetch_eula.rb | 6 +++--- app/sidekiq/tii_check_progress_job.rb | 11 ++++++++++- config/initializers/log_initializer.rb | 2 +- config/schedule.yml | 2 +- ...ails.rb => 20240105055902_add_tii_details.rb} | 5 +++-- db/schema.rb | 11 +++++------ 7 files changed, 33 insertions(+), 20 deletions(-) rename db/migrate/{20230420224824_add_tii_details.rb => 20240105055902_add_tii_details.rb} (95%) diff --git a/app/models/turn_it_in/tii_action.rb b/app/models/turn_it_in/tii_action.rb index d1de7ec80..f4692ec77 100644 --- a/app/models/turn_it_in/tii_action.rb +++ b/app/models/turn_it_in/tii_action.rb @@ -135,8 +135,12 @@ def save_and_mark_complete save! end + # Record that we have completed a part of the action + # Resets the retry count, and mark the part complete at time + # to ensure future retries start from the new time. def save_progress self.retries = 0 + self.complete_at = DateTime.now save! end @@ -147,17 +151,17 @@ def run raise "SYSTEM ERROR: method missing" end - # Retry the request in 30 minutes, up to 10 times + # Retry the request up to 48 times over 24 hours. + # The TiiCheckProgressJob will retry any actions that have not been completed def retry_request self.retries += 1 - if self.retries > 10 + last_recorded_complete_time = complete_at || created_at + + if self.retries > 48 && last_recorded_complete_time < DateTime.now - 24.hours self.error_code = :excessive_retries else self.retry = true - - # We try in 15 minutes - TiiActionJob.perform_at(Time.zone.now + 15.minutes, id) end end @@ -173,7 +177,7 @@ def handle_error(error, codes = []) when 451 self.error_cde = :no_user_with_accepted_eula return - when 429 + when 429 # is not currently used by TCA self.error_code = :rate_limited retry_request return diff --git a/app/models/turn_it_in/tii_action_fetch_eula.rb b/app/models/turn_it_in/tii_action_fetch_eula.rb index b2c4c9835..b23d3b0fd 100644 --- a/app/models/turn_it_in/tii_action_fetch_eula.rb +++ b/app/models/turn_it_in/tii_action_fetch_eula.rb @@ -16,11 +16,11 @@ def run # Check if an update of the eula is required def update_required? - last_feature_check = last_run + last_eula_check = last_run !Rails.cache.exist?('tii.eula_version') || - last_feature_check.nil? || - last_feature_check < DateTime.now - 1.day + last_eula_check.nil? || + last_eula_check < DateTime.now - 1.day end # Connect to tii to get the latest eula details. diff --git a/app/sidekiq/tii_check_progress_job.rb b/app/sidekiq/tii_check_progress_job.rb index 26d06936e..0a042ba92 100644 --- a/app/sidekiq/tii_check_progress_job.rb +++ b/app/sidekiq/tii_check_progress_job.rb @@ -16,6 +16,15 @@ def run_waiting_actions # Get the actions waiting to retry, where last run is more than 30 minutes ago, and run them TiiAction.where(retry: true, complete: false) .where('(last_run IS NULL AND created_at < :date) OR last_run < :date', date: DateTime.now - 30.minutes) - .each(&:perform) + .each do |action| + action.perform + + # Stop if the service is not available + break if action.status == :service_not_available + + # Sleep to ensure requests are performed at a rate of well below 100 per minute + sleep(2) + end end end + diff --git a/config/initializers/log_initializer.rb b/config/initializers/log_initializer.rb index 302716183..9748142b7 100644 --- a/config/initializers/log_initializer.rb +++ b/config/initializers/log_initializer.rb @@ -9,7 +9,7 @@ class FormatifFormatter < Logger::Formatter # This method is invoked when a log event occurs def call(severity, timestamp, _progname, msg) remote_ip = Thread.current.thread_variable_get(:ip) || 'unknown' - "#{timestamp},#{remote_ip},#{severity}: #{msg.gsub(/\n/, '\n')}\n" + "#{timestamp},#{remote_ip},#{severity}: #{msg.to_s.gsub(/\n/, '\n')}\n" end end diff --git a/config/schedule.yml b/config/schedule.yml index ca5aad30d..22c423b2c 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -5,5 +5,5 @@ register_webhooks: class: "TiiRegisterWebHookJob" progress_turn_it_in_jobs: - cron: "every 3 hours" + cron: "every 30 minutes" class: "TiiCheckProgressJob" diff --git a/db/migrate/20230420224824_add_tii_details.rb b/db/migrate/20240105055902_add_tii_details.rb similarity index 95% rename from db/migrate/20230420224824_add_tii_details.rb rename to db/migrate/20240105055902_add_tii_details.rb index 803da4382..12b89a2ca 100644 --- a/db/migrate/20230420224824_add_tii_details.rb +++ b/db/migrate/20240105055902_add_tii_details.rb @@ -1,4 +1,4 @@ -class AddTiiDetails < ActiveRecord::Migration[7.0] +class AddTiiDetails < ActiveRecord::Migration[7.1] def change add_column :users, :tii_eula_version, :string add_column :users, :tii_eula_date, :datetime @@ -27,7 +27,7 @@ def change t.bigint :submitted_by_user_id, null: false t.index :submitted_by_user_id t.string :filename, null: false - t.integer :idx, nul: false + t.integer :idx, null: false t.string :submission_id t.string :similarity_pdf_id @@ -64,6 +64,7 @@ def change t.integer :retries, default: 0, null: false t.datetime :last_run + t.datetime :complete_at t.boolean :retry, default: true, null: false t.integer :error_code diff --git a/db/schema.rb b/db/schema.rb index 319b39262..0f10c135b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2023_12_01_000245) do +ActiveRecord::Schema[7.1].define(version: 2024_01_05_055902) do create_table "activity_types", charset: "utf8", collation: "utf8_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "abbreviation", null: false @@ -234,10 +234,10 @@ t.datetime "created_at", null: false t.datetime "updated_at", null: false t.string "abbreviation" - t.text "upload_requirements", size: :long, collation: "utf8mb4_bin" + t.string "upload_requirements", limit: 4096 t.integer "target_grade", default: 0 t.boolean "restrict_status_updates", default: false - t.text "plagiarism_checks", size: :long, collation: "utf8mb4_bin" + t.string "plagiarism_checks", limit: 4096 t.string "plagiarism_report_url" t.boolean "plagiarism_updated", default: false t.integer "plagiarism_warn_pct", default: 50 @@ -255,8 +255,6 @@ t.index ["overseer_image_id"], name: "index_task_definitions_on_overseer_image_id" t.index ["tutorial_stream_id"], name: "index_task_definitions_on_tutorial_stream_id" t.index ["unit_id"], name: "index_task_definitions_on_unit_id" - t.check_constraint "json_valid(`plagiarism_checks`)", name: "plagiarism_checks" - t.check_constraint "json_valid(`upload_requirements`)", name: "upload_requirements" end create_table "task_engagements", charset: "utf8", collation: "utf8_unicode_ci", force: :cascade do |t| @@ -354,6 +352,7 @@ t.boolean "complete", default: false, null: false t.integer "retries", default: 0, null: false t.datetime "last_run" + t.datetime "complete_at" t.boolean "retry", default: true, null: false t.integer "error_code" t.text "custom_error_message" @@ -384,7 +383,7 @@ t.bigint "tii_task_similarity_id" t.bigint "submitted_by_user_id", null: false t.string "filename", null: false - t.integer "idx" + t.integer "idx", null: false t.string "submission_id" t.string "similarity_pdf_id" t.datetime "submitted_at"