diff --git a/app/controllers/clock_ins_controller.rb b/app/controllers/clock_ins_controller.rb index 448ffc5..dbcb8b6 100644 --- a/app/controllers/clock_ins_controller.rb +++ b/app/controllers/clock_ins_controller.rb @@ -1,14 +1,16 @@ class ClockInsController < ApplicationController include UserFinder + include Retryable def upsert current_time = Time.now - begin + with_retry do ActiveRecord::Base.transaction do last_unused_clock_in = ClockIn.where( - user_id: @user.id, - is_clocked_out: false) + is_clocked_out: false, + user_id: @user.id + ) .order(clocked_in_at: :desc) .limit(1) .lock(true) @@ -27,14 +29,12 @@ def upsert else ClockIn.create!( user_id: @user.id, - clocked_in_at: Time.now + clocked_in_at: current_time ) render_success(@user.clock_ins_json, I18n.t('success.messages.clock_in_success')) end end - rescue ActiveRecord::ActiveRecordError => e - render_error(:unprocessable_entity, e.message) end end diff --git a/app/controllers/concerns/retryable.rb b/app/controllers/concerns/retryable.rb new file mode 100644 index 0000000..b738539 --- /dev/null +++ b/app/controllers/concerns/retryable.rb @@ -0,0 +1,28 @@ +module Retryable + MAX_RETRIES = 3 + RETRY_DELAY = 0.5 # seconds + + def with_retry(max_retries: MAX_RETRIES, delay: RETRY_DELAY) + attempts = 0 + + begin + yield + rescue ActiveRecord::Deadlocked => e + attempts += 1 + Rails.logger.warn("Retryable Error: #{e.class} - #{e.message}. Attempt #{attempts} of #{max_retries}.") + if attempts < max_retries + sleep(delay * attempts) + retry + else + Rails.logger.error("Retries exhausted for #{e.class}: #{e.message}") + raise e + end + rescue ActiveRecord::ActiveRecordError => e + Rails.logger.error("ActiveRecord Error: #{e.class} - #{e.message}") + render_error(:unprocessable_entity, e.message) + rescue StandardError => e + Rails.logger.error("Unexpected Error: #{e.class} - #{e.message}") + render_error(:internal_server_error, e.message) + end + end +end \ No newline at end of file diff --git a/db/migrate/20250102060751_add_index_to_user_follows_on_follower_and_followee.rb b/db/migrate/20250102060751_add_index_to_user_follows_on_follower_and_followee.rb index f415519..36b627f 100644 --- a/db/migrate/20250102060751_add_index_to_user_follows_on_follower_and_followee.rb +++ b/db/migrate/20250102060751_add_index_to_user_follows_on_follower_and_followee.rb @@ -4,6 +4,6 @@ def up end def down - add_index :user_follows, name: 'index_user_follows_on_follower_id_and_followee_id' + remove_index :user_follows, name: 'index_user_follows_on_follower_id_and_followee_id' end end diff --git a/db/migrate/20250103014113_add_index_clock_ins_on_is_clocked_out_and_user_id.rb b/db/migrate/20250103014113_add_index_clock_ins_on_is_clocked_out_and_user_id.rb new file mode 100644 index 0000000..be65c23 --- /dev/null +++ b/db/migrate/20250103014113_add_index_clock_ins_on_is_clocked_out_and_user_id.rb @@ -0,0 +1,11 @@ +class AddIndexClockInsOnIsClockedOutAndUserId < ActiveRecord::Migration[7.0] + def up + add_index :clock_ins, [:is_clocked_out, :user_id], name: 'index_clock_ins_on_is_clocked_out_and_user_id' + remove_index :clock_ins, [:user_id, :clocked_in_at] + end + + def down + remove_index :user_follows, name: 'index_clock_ins_on_is_clocked_out_and_user_id' + add_index :clock_ins, [:user_id, :clocked_in_at] + end +end diff --git a/db/schema.rb b/db/schema.rb index bfe3597..7cfcca2 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.0].define(version: 2025_01_02_060751) do +ActiveRecord::Schema[7.0].define(version: 2025_01_03_014113) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -18,7 +18,7 @@ t.bigint "user_id", null: false t.datetime "clocked_in_at", null: false t.boolean "is_clocked_out", default: false, null: false - t.index ["user_id", "clocked_in_at"], name: "index_clock_ins_on_user_id_and_clocked_in_at" + t.index ["is_clocked_out", "user_id"], name: "index_clock_ins_on_is_clocked_out_and_user_id" t.index ["user_id"], name: "index_clock_ins_on_user_id" end diff --git a/system-design/README.md b/system-design/README.md index ece3760..1c27ae7 100644 --- a/system-design/README.md +++ b/system-design/README.md @@ -23,6 +23,7 @@ This document contains the details of ideas and implementation for time tracker 3. For any clock-ins, every second clock-in will be considered a clock out for the last clock-in for us to calculate sleep record. 4. Since the feature is quite simple, most part is enough with simple error message, but it is better to implement proper logging. 5. Need proper error handling because ActiveModel::Error's full_messages return message with key. +6. Need to implement retry mechanism for concurrent requests to mitigate deadlocks on locking. ## Non-Functional Requirements 1. **Scalability**