Commit 4264e9d8 authored by Adam Hegyi's avatar Adam Hegyi

Schedule user status cleanup

This MR implements scheduled user status cleanup.

- Adding `clear_status_at` column to `user_statuses`
- Add index to `clear_status_at` to easily query the statuses that needs
  cleanup
- Add cronjob that looks for relevant records and invokes the cleanup
parent 82cd8d68
...@@ -7,6 +7,16 @@ class UserStatus < ApplicationRecord ...@@ -7,6 +7,16 @@ class UserStatus < ApplicationRecord
DEFAULT_EMOJI = 'speech_balloon' DEFAULT_EMOJI = 'speech_balloon'
CLEAR_STATUS_QUICK_OPTIONS = {
'30_minutes' => 30.minutes,
'3_hours' => 3.hours,
'8_hours' => 8.hours,
'1_day' => 1.day,
'3_days' => 3.days,
'7_days' => 7.days,
'30_days' => 30.days
}.freeze
belongs_to :user belongs_to :user
enum availability: { not_set: 0, busy: 1 } enum availability: { not_set: 0, busy: 1 }
...@@ -15,5 +25,11 @@ class UserStatus < ApplicationRecord ...@@ -15,5 +25,11 @@ class UserStatus < ApplicationRecord
validates :emoji, inclusion: { in: Gitlab::Emoji.emojis_names } validates :emoji, inclusion: { in: Gitlab::Emoji.emojis_names }
validates :message, length: { maximum: 100 }, allow_blank: true validates :message, length: { maximum: 100 }, allow_blank: true
scope :scheduled_for_cleanup, -> { where(arel_table[:clear_status_at].lteq(Time.current)) }
cache_markdown_field :message, pipeline: :emoji cache_markdown_field :message, pipeline: :emoji
def clear_status_after=(value)
self.clear_status_at = CLEAR_STATUS_QUICK_OPTIONS[value]&.from_now
end
end end
# frozen_string_literal: true
module Users
class BatchStatusCleanerService
BATCH_SIZE = 100.freeze
# Cleanup BATCH_SIZE user_statuses records
# rubocop: disable CodeReuse/ActiveRecord
def self.execute(batch_size: BATCH_SIZE)
scope = UserStatus
.select(:user_id)
.scheduled_for_cleanup
.lock('FOR UPDATE SKIP LOCKED')
.limit(batch_size)
deleted_rows = UserStatus.where(user_id: scope).delete_all
{ deleted_rows: deleted_rows }
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
...@@ -467,6 +467,14 @@ ...@@ -467,6 +467,14 @@
:weight: 1 :weight: 1
:idempotent: true :idempotent: true
:tags: [] :tags: []
- :name: cronjob:user_status_cleanup_batch
:feature_category: :users
:has_external_dependencies:
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent: true
:tags: []
- :name: cronjob:users_create_statistics - :name: cronjob:users_create_statistics
:feature_category: :users :feature_category: :users
:has_external_dependencies: :has_external_dependencies:
......
# frozen_string_literal: true
module UserStatusCleanup
# This worker will run every minute to look for user status records to clean up.
class BatchWorker
include ApplicationWorker
# rubocop:disable Scalability/CronWorkerContext
include CronjobQueue
# rubocop:enable Scalability/CronWorkerContext
feature_category :users
idempotent!
# Avoid running too many UPDATE queries at once
MAX_RUNTIME = 30.seconds
def perform
return unless UserStatus.scheduled_for_cleanup.exists?
start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
loop do
result = Users::BatchStatusCleanerService.execute
break if result[:deleted_rows] < Users::BatchStatusCleanerService::BATCH_SIZE
current_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
break if (current_time - start_time) > MAX_RUNTIME
end
end
end
end
---
title: Add clear_status_at column to user_status table
merge_request: 53620
author:
type: other
---
name: clear_status_with_quick_options
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53620
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320777
milestone: '13.9'
type: development
group: group::optimize
default_enabled: false
...@@ -553,6 +553,9 @@ Settings.cron_jobs['schedule_merge_request_cleanup_refs_worker']['job_class'] = ...@@ -553,6 +553,9 @@ Settings.cron_jobs['schedule_merge_request_cleanup_refs_worker']['job_class'] =
Settings.cron_jobs['manage_evidence_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['manage_evidence_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['manage_evidence_worker']['cron'] ||= '0 * * * *' Settings.cron_jobs['manage_evidence_worker']['cron'] ||= '0 * * * *'
Settings.cron_jobs['manage_evidence_worker']['job_class'] = 'Releases::ManageEvidenceWorker' Settings.cron_jobs['manage_evidence_worker']['job_class'] = 'Releases::ManageEvidenceWorker'
Settings.cron_jobs['user_status_cleanup_batch_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['user_status_cleanup_batch_worker']['cron'] ||= '* * * * *'
Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatusCleanup::BatchWorker'
Gitlab.com do Gitlab.com do
Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({})
......
# frozen_string_literal: true
class AddStatusExpiresAtToUserStatuses < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_column(:user_statuses, :clear_status_at, :datetime_with_timezone, null: true)
end
end
def down
with_lock_retries do
remove_column(:user_statuses, :clear_status_at)
end
end
end
# frozen_string_literal: true
class AddIndexOnUserStatusesStatusExpiresAt < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
INDEX_NAME = 'index_user_statuses_on_clear_status_at_not_null'
disable_ddl_transaction!
def up
add_concurrent_index(:user_statuses, :clear_status_at, name: INDEX_NAME, where: 'clear_status_at IS NOT NULL')
end
def down
remove_concurrent_index_by_name(:user_statuses, INDEX_NAME)
end
end
b9200d6c754f7c450ba0c718171806e8f4f9720d870e532f4800640ca707f24f
\ No newline at end of file
3a7fb1b7959f09b9ba464253a72d52bcb744e7f78aac4f44e1d9201fa3c8387d
\ No newline at end of file
...@@ -17842,7 +17842,8 @@ CREATE TABLE user_statuses ( ...@@ -17842,7 +17842,8 @@ CREATE TABLE user_statuses (
emoji character varying DEFAULT 'speech_balloon'::character varying NOT NULL, emoji character varying DEFAULT 'speech_balloon'::character varying NOT NULL,
message character varying(100), message character varying(100),
message_html character varying, message_html character varying,
availability smallint DEFAULT 0 NOT NULL availability smallint DEFAULT 0 NOT NULL,
clear_status_at timestamp with time zone
); );
CREATE SEQUENCE user_statuses_user_id_seq CREATE SEQUENCE user_statuses_user_id_seq
...@@ -23521,6 +23522,8 @@ CREATE INDEX index_user_preferences_on_gitpod_enabled ON user_preferences USING ...@@ -23521,6 +23522,8 @@ CREATE INDEX index_user_preferences_on_gitpod_enabled ON user_preferences USING
CREATE UNIQUE INDEX index_user_preferences_on_user_id ON user_preferences USING btree (user_id); CREATE UNIQUE INDEX index_user_preferences_on_user_id ON user_preferences USING btree (user_id);
CREATE INDEX index_user_statuses_on_clear_status_at_not_null ON user_statuses USING btree (clear_status_at) WHERE (clear_status_at IS NOT NULL);
CREATE INDEX index_user_statuses_on_user_id ON user_statuses USING btree (user_id); CREATE INDEX index_user_statuses_on_user_id ON user_statuses USING btree (user_id);
CREATE UNIQUE INDEX index_user_synced_attributes_metadata_on_user_id ON user_synced_attributes_metadata USING btree (user_id); CREATE UNIQUE INDEX index_user_synced_attributes_metadata_on_user_id ON user_synced_attributes_metadata USING btree (user_id);
...@@ -9,6 +9,7 @@ module API ...@@ -9,6 +9,7 @@ module API
expose :message_html do |entity| expose :message_html do |entity|
MarkupHelper.markdown_field(entity, :message) MarkupHelper.markdown_field(entity, :message)
end end
expose :clear_status_at
end end
end end
end end
...@@ -1004,11 +1004,15 @@ module API ...@@ -1004,11 +1004,15 @@ module API
optional :emoji, type: String, desc: "The emoji to set on the status" optional :emoji, type: String, desc: "The emoji to set on the status"
optional :message, type: String, desc: "The status message to set" optional :message, type: String, desc: "The status message to set"
optional :availability, type: String, desc: "The availability of user to set" optional :availability, type: String, desc: "The availability of user to set"
optional :clear_status_after, type: String, desc: "Automatically clear emoji, message and availability fields after a certain time", values: UserStatus::CLEAR_STATUS_QUICK_OPTIONS.keys
end end
put "status", feature_category: :users do put "status", feature_category: :users do
forbidden! unless can?(current_user, :update_user_status, current_user) forbidden! unless can?(current_user, :update_user_status, current_user)
if ::Users::SetStatusService.new(current_user, declared_params).execute update_params = declared_params
update_params.delete(:clear_status_after) if Feature.disabled?(:clear_status_with_quick_options, current_user)
if ::Users::SetStatusService.new(current_user, update_params).execute
present current_user.status, with: Entities::UserStatus present current_user.status, with: Entities::UserStatus
else else
render_validation_error!(current_user.status) render_validation_error!(current_user.status)
......
...@@ -17,4 +17,34 @@ RSpec.describe UserStatus do ...@@ -17,4 +17,34 @@ RSpec.describe UserStatus do
expect { status.user.destroy }.to change { described_class.count }.from(1).to(0) expect { status.user.destroy }.to change { described_class.count }.from(1).to(0)
end end
describe '#clear_status_after=' do
it 'sets clear_status_at' do
status = build(:user_status)
freeze_time do
status.clear_status_after = '8_hours'
expect(status.clear_status_at).to be_like_time(8.hours.from_now)
end
end
it 'unsets clear_status_at' do
status = build(:user_status, clear_status_at: 8.hours.from_now)
status.clear_status_after = nil
expect(status.clear_status_at).to be_nil
end
context 'when unknown clear status is given' do
it 'unsets clear_status_at' do
status = build(:user_status, clear_status_at: 8.hours.from_now)
status.clear_status_after = 'unknown'
expect(status.clear_status_at).to be_nil
end
end
end
end end
...@@ -2866,6 +2866,47 @@ RSpec.describe API::Users do ...@@ -2866,6 +2866,47 @@ RSpec.describe API::Users do
expect(response).to have_gitlab_http_status(:success) expect(response).to have_gitlab_http_status(:success)
expect(user.reload.status).to be_nil expect(user.reload.status).to be_nil
end end
context 'when clear_status_after is given' do
it 'sets the clear_status_at column' do
freeze_time do
expected_clear_status_at = 3.hours.from_now
put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' }
expect(response).to have_gitlab_http_status(:success)
expect(user.status.reload.clear_status_at).to be_within(1.minute).of(expected_clear_status_at)
expect(Time.parse(json_response["clear_status_at"])).to be_within(1.minute).of(expected_clear_status_at)
end
end
it 'unsets the clear_status_at column' do
user.create_status!(clear_status_at: 5.hours.ago)
put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: nil }
expect(response).to have_gitlab_http_status(:success)
expect(user.status.reload.clear_status_at).to be_nil
end
it 'raises error when unknown status value is given' do
put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: 'wrong' }
expect(response).to have_gitlab_http_status(:bad_request)
end
context 'when the clear_status_with_quick_options feature flag is disabled' do
before do
stub_feature_flags(clear_status_with_quick_options: false)
end
it 'does not persist clear_status_at' do
put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' }
expect(user.status.reload.clear_status_at).to be_nil
end
end
end
end end
describe 'POST /users/:user_id/personal_access_tokens' do describe 'POST /users/:user_id/personal_access_tokens' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Users::BatchStatusCleanerService do
let_it_be(:user_status_1) { create(:user_status, emoji: 'coffee', message: 'msg1', clear_status_at: 1.year.ago) }
let_it_be(:user_status_2) { create(:user_status, emoji: 'coffee', message: 'msg1', clear_status_at: 1.year.from_now) }
let_it_be(:user_status_3) { create(:user_status, emoji: 'coffee', message: 'msg1', clear_status_at: 2.years.ago) }
let_it_be(:user_status_4) { create(:user_status, emoji: 'coffee', message: 'msg1') }
subject(:result) { described_class.execute }
it 'cleans up scheduled user statuses' do
expect(result[:deleted_rows]).to eq(2)
deleted_statuses = UserStatus.where(user_id: [user_status_1.user_id, user_status_3.user_id])
expect(deleted_statuses).to be_empty
end
it 'does not affect rows with future clear_status_at' do
expect { result }.not_to change { user_status_2.reload }
end
it 'does not affect rows without clear_status_at' do
expect { result }.not_to change { user_status_4.reload }
end
describe 'batch_size' do
it 'clears status in batches' do
result = described_class.execute(batch_size: 1)
expect(result[:deleted_rows]).to eq(1)
result = described_class.execute(batch_size: 1)
expect(result[:deleted_rows]).to eq(1)
result = described_class.execute(batch_size: 1)
expect(result[:deleted_rows]).to eq(0)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe UserStatusCleanup::BatchWorker do
include_examples 'an idempotent worker' do
subject do
perform_multiple([], worker: described_class.new)
end
end
describe '#perform' do
subject(:run_worker) { described_class.new.perform }
context 'when no records are scheduled for cleanup' do
let(:user_status) { create(:user_status) }
it 'does nothing' do
expect { run_worker }.not_to change { user_status.reload }
end
end
it 'cleans up the records' do
user_status_1 = create(:user_status, clear_status_at: 1.year.ago)
user_status_2 = create(:user_status, clear_status_at: 2.years.ago)
run_worker
deleted_statuses = UserStatus.where(user_id: [user_status_1.user_id, user_status_2.user_id])
expect(deleted_statuses).to be_empty
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment