From b8cd6f66cf456098e603255ac15245110aacd6ca Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Thu, 19 Dec 2024 19:25:43 +0200 Subject: [PATCH] FIX: Split BadgeGrant in a job for each badge This should keep the execution time of BadgeGrant low and avoid clogging the Sidekiq queue. --- app/jobs/regular/ensure_badge_consistency.rb | 13 ++++++++ app/jobs/regular/grant_badge.rb | 34 ++++++++++++++++++++ app/jobs/scheduled/badge_grant.rb | 31 ------------------ app/jobs/scheduled/grant_all_badges.rb | 13 ++++++++ 4 files changed, 60 insertions(+), 31 deletions(-) create mode 100644 app/jobs/regular/ensure_badge_consistency.rb create mode 100644 app/jobs/regular/grant_badge.rb delete mode 100644 app/jobs/scheduled/badge_grant.rb create mode 100644 app/jobs/scheduled/grant_all_badges.rb diff --git a/app/jobs/regular/ensure_badge_consistency.rb b/app/jobs/regular/ensure_badge_consistency.rb new file mode 100644 index 00000000000..756a8160953 --- /dev/null +++ b/app/jobs/regular/ensure_badge_consistency.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Jobs + class EnsureBadgeConsistency < ::Jobs::Base + def execute(args) + return unless SiteSetting.enable_badges + + BadgeGranter.revoke_ungranted_titles! + UserBadge.ensure_consistency! # Badge granter sometimes uses raw SQL, so hooks do not run. Clean up data + UserStat.update_distinct_badge_count + end + end +end diff --git a/app/jobs/regular/grant_badge.rb b/app/jobs/regular/grant_badge.rb new file mode 100644 index 00000000000..7c411acd068 --- /dev/null +++ b/app/jobs/regular/grant_badge.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Jobs + class GrantBadge < ::Jobs::Base + def execute(args) + return unless SiteSetting.enable_badges + + badge = Badge.enabled.find_by(id: args[:badge_id]) + return unless badge + + # Cancels the scheduled job to ensure badge consistency as the badges are + # mutating during `BadgeGranter.backfill`. + Jobs.cancel_scheduled_job(:ensure_badge_consistency) + + begin + BadgeGranter.backfill(badge) + rescue => ex + # TODO - expose errors in UI + Discourse.handle_job_exception( + ex, + error_context({}, code_desc: "Exception granting badges", extra: { badge_id: badge.id }), + ) + end + + # Re-schedule the job in the future to allow all GrantBadge jobs to start + # and thus ensuring this job runs only once after all badges scheduled by + # GrantAllBadges have been granted. + DistributedMutex.synchronize("ensure_badge_consistency") do + Jobs.cancel_scheduled_job(:ensure_badge_consistency) + Jobs.enqueue_in(5.minutes, :ensure_badge_consistency) + end + end + end +end diff --git a/app/jobs/scheduled/badge_grant.rb b/app/jobs/scheduled/badge_grant.rb deleted file mode 100644 index b9bc1ad4e82..00000000000 --- a/app/jobs/scheduled/badge_grant.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module Jobs - class BadgeGrant < ::Jobs::Scheduled - def self.run - self.new.execute(nil) - end - - every 1.day - - def execute(args) - return unless SiteSetting.enable_badges - - Badge.enabled.find_each do |b| - begin - BadgeGranter.backfill(b) - rescue => ex - # TODO - expose errors in UI - Discourse.handle_job_exception( - ex, - error_context({}, code_desc: "Exception granting badges", extra: { badge_id: b.id }), - ) - end - end - - BadgeGranter.revoke_ungranted_titles! - UserBadge.ensure_consistency! # Badge granter sometimes uses raw SQL, so hooks do not run. Clean up data - UserStat.update_distinct_badge_count - end - end -end diff --git a/app/jobs/scheduled/grant_all_badges.rb b/app/jobs/scheduled/grant_all_badges.rb new file mode 100644 index 00000000000..716646660b0 --- /dev/null +++ b/app/jobs/scheduled/grant_all_badges.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Jobs + class GrantAllBadges < ::Jobs::Scheduled + every 1.day + + def execute(args) + return unless SiteSetting.enable_badges + + Badge.enabled.find_each { |b| Jobs.enqueue(:grant_badge, badge_id: b.id) } + end + end +end