Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.1.6
2.1.7
9 changes: 8 additions & 1 deletion app/jobs/concerns/sync_attempt_manageable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ module SyncAttemptManageable
private

def find_or_create_sync_attempt(source, attempt_id)
attempt_id ? SyncAttempt.find(attempt_id) : SyncAttempt.create!(calendar_source: source, status: :queued)
return SyncAttempt.find(attempt_id) if attempt_id

SyncAttempt.create!(calendar_source: source, status: :queued)
rescue ActiveRecord::RecordNotUnique
# Another worker already created the active attempt for this source
# (enforced by idx_unique_active_sync_attempt_per_source) -- reuse it
# instead of crashing the job.
source.sync_attempts.where(status: ["queued", "running"]).order(created_at: :desc).first || raise
end
end
10 changes: 9 additions & 1 deletion app/services/calendar_hub/auto_sync_scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,15 @@ def schedule_syncs(sources)

schedule.each do |source_id, scheduled_at|
source = sources_by_id[source_id] || CalendarSource.find(source_id)
attempt = SyncAttempt.create!(calendar_source: source, status: :queued)

begin
attempt = SyncAttempt.create!(calendar_source: source, status: :queued)
rescue ActiveRecord::RecordNotUnique
# Another worker already created an active attempt for this source
# (enforced by idx_unique_active_sync_attempt_per_source) between
# find_sources_due_for_sync and here -- skip it, it's already scheduled.
next
end

if scheduled_at <= @now
SyncCalendarJob.perform_later(source.id, attempt_id: attempt.id)
Expand Down
16 changes: 16 additions & 0 deletions test/jobs/sync_calendar_job_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,22 @@ class SyncCalendarJobTest < ActiveJob::TestCase
assert(source.sync_attempts.exists?(status: "success"))
end

test "reuses existing active attempt when concurrent creation hits the unique constraint" do
source = calendar_sources(:provider)
existing_attempt = source.sync_attempts.create!(status: :queued)

# Simulate another worker winning the race to create the active attempt
# for this source (enforced by idx_unique_active_sync_attempt_per_source).
SyncAttempt.expects(:create!).with(calendar_source: source, status: :queued)
.raises(ActiveRecord::RecordNotUnique.new("UNIQUE constraint failed"))

CalendarHub::Sync::EnhancedSyncService.expects(:new).with(source: source, observer: existing_attempt).returns(mock(call: true))

SyncCalendarJob.perform_now(source.id)

assert_equal "success", existing_attempt.reload.status
end

# FEAT-006: Sync failure tracking

test "records sync success and resets consecutive_sync_failures" do
Expand Down
22 changes: 22 additions & 0 deletions test/services/calendar_hub/auto_sync_scheduler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,28 @@ class AutoSyncSchedulerTest < ActiveJob::TestCase
assert_includes(due_sources, @source2)
end

test "schedule_syncs skips a source when a concurrent attempt already exists" do
@source1.update!(last_synced_at: 1.hour.ago)
@source2.update!(last_synced_at: 1.hour.ago)

scheduler = ::CalendarHub::AutoSyncScheduler.new
sources = scheduler.find_sources_due_for_sync

assert_equal(2, sources.count)

# Simulate another worker winning the race to create the active attempt
# for source1 between find_sources_due_for_sync and schedule_syncs -- the
# real DB constraint (idx_unique_active_sync_attempt_per_source) is what
# schedule_syncs must tolerate.
SyncAttempt.create!(calendar_source: @source1, status: :queued)

assert_enqueued_jobs(1, only: SyncCalendarJob) do
result = scheduler.schedule_syncs(sources)

assert_equal(1, result)
end
end

test "schedule_syncs does not re-fetch sources already in memory" do
@source1.update!(last_synced_at: 1.hour.ago)
@source2.update!(last_synced_at: 1.hour.ago)
Expand Down