Schedule user status cleanup [RUN ALL RSPEC] [RUN AS-IF-FOSS]
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 touser_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?
- User sets the status via the status popup
- User selects a pre-defined quick option, see:
Gitlab::UserStatusCleanup::QuickOptions
-
clear_status_at
field is filled - A cron job (running every minute) looks for user statuses to clean up
- 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 theDELETE
statement - The background job can run for 30 seconds maximum
- We expect small amount of
DELETE
-s, we have about 150Kuser_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
- Exists query: https://explain.depesz.com/s/lDm9
- DELETE query: https://explain.depesz.com/s/lEjl
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
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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