Skip to content

Schedule user status cleanup [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Adam Hegyi requested to merge 262086-schedule-user-unset-status into master

What does this MR do?

This MR implements the backend part of scheduled user status cleanup. Note that the API is not user facing yet (behind FF).

The change is behind a feature flag: clear_status_with_quick_options The FF can be enabled for a particular user.

  • Accept clear_status_after param on the BE (behind FF)
  • 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

How will it work?

image

  1. User sets the status via the status popup
  2. User selects a pre-defined quick option, see: Gitlab::UserStatusCleanup::QuickOptions
  3. clear_status_at field is filled
  4. A cron job (running every minute) looks for user statuses to clean up
  5. The cron job deletes the user_statuses records (same behavior when the user clears the status)

Prevent long runtime and locking:

  • Use FOR UPDATE SKIP LOCKED when executing the DELETE statement
  • The background job can run for 30 seconds maximum
  • We expect small amount of DELETE-s, we have about 150K user_statuses records atm.

Queries

Testing

Generating 1000 user statuses with clear_status_at filled in:

with cte as (
select user_id, timestamp from user_statuses inner join (select timestamp '2021-01-10 20:00:00' + random() * (timestamp '2021-03-01 20:00:00' - timestamp '2021-01-10 10:00:00') as timestamp FROM generate_series(1, 1000)) as timestamps on true order by random() limit 1000
)
update user_statuses set clear_status_at=cte.timestamp from cte where cte.user_id=user_statuses.user_id;

In rails console:

UserStatusCleanup::BatchWorker.new.perform

API for using the quick options

curl --request PUT --header "PRIVATE-TOKEN: YOUR_TOKEN" --data "clear_status_after=30_minutes" --data "emoji=coffee" --data "message=I crave coffee" "http://localhost:3000/api/v4/user/status"

Migration

Up:

== 20210128101707 AddPreventMergeWithoutJiraIssueToProjectSettings: migrating =
-- add_column(:project_settings, :prevent_merge_without_jira_issue, :boolean, {:null=>false, :default=>false})
   -> 0.0014s
== 20210128101707 AddPreventMergeWithoutJiraIssueToProjectSettings: migrated (0.0035s)

== 20210208125050 AddStatusExpiresAtToUserStatuses: migrating =================
-- add_column(:user_statuses, :clear_status_at, :datetime_with_timezone, {:null=>true})
   -> 0.0011s
== 20210208125050 AddStatusExpiresAtToUserStatuses: migrated (0.0025s) ========

== 20210208125248 AddIndexOnUserStatusesStatusExpiresAt: migrating ============
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:user_statuses, :clear_status_at, {:name=>"index_user_statuses_on_clear_status_at_not_null", :where=>"clear_status_at IS NOT NULL", :algorithm=>:concurrently})
   -> 0.0014s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- add_index(:user_statuses, :clear_status_at, {:name=>"index_user_statuses_on_clear_status_at_not_null", :where=>"clear_status_at IS NOT NULL", :algorithm=>:concurrently})
   -> 0.0109s
-- execute("RESET ALL")
   -> 0.0002s
== 20210208125248 AddIndexOnUserStatusesStatusExpiresAt: migrated (0.0131s) ===

Down

== 20210208125248 AddIndexOnUserStatusesStatusExpiresAt: reverting ============
-- transaction_open?()
   -> 0.0000s
-- indexes(:user_statuses)
   -> 0.0019s
-- execute("SET statement_timeout TO 0")
   -> 0.0002s
-- remove_index(:user_statuses, {:algorithm=>:concurrently, :name=>"index_user_statuses_on_clear_status_at_not_null"})
   -> 0.0023s
-- execute("RESET ALL")
   -> 0.0002s
== 20210208125248 AddIndexOnUserStatusesStatusExpiresAt: reverted (0.0050s) ===
== 20210208125050 AddStatusExpiresAtToUserStatuses: reverting =================
-- remove_column(:user_statuses, :clear_status_at)
   -> 0.0006s
== 20210208125050 AddStatusExpiresAtToUserStatuses: reverted (0.0025s) ========

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to #262086 (closed)

Edited by Adam Hegyi

Merge request reports