Skip to content

Remove references to alerts service

Sarah Yasonik requested to merge sy-remove-alerts-service into master

What does this MR do?

The aim of this MR is to begin the removal of AlertsService, as it has been replaced by AlertManagement::HttpIntegration.

This will be done in three stages:

  1. In %13.8, remove ability to add new AlertsService records - !50794 (merged), !50525 (merged)
  2. In %13.8, remove AlertsService records from the services table & remove the corresponding alerts service logic/views - !50913 (merged)
  3. In %13.9, remove the AlertsService class & drop the alerts_service_data table. #296923 (closed)

^^ I roughly based this process on a comparable removal & advice from #database.

Supplemental database info

Migration output
$ rails db:migrate
== 20210107194543 RemoveAlertsServiceRecords: migrating =======================
== 20210107194543 RemoveAlertsServiceRecords: migrated (0.0169s) ==============
$ rails db:rollback
== 20210107194543 RemoveAlertsServiceRecords: reverting =======================
== 20210107194543 RemoveAlertsServiceRecords: reverted (0.0000s) ==============
Operation performance

Query:

DELETE FROM "services" WHERE "services"."type" = 'AlertsService'

Visualization: https://explain.depesz.com/s/goc2

From #database-lab:

Summary:
Time: 1.429 s
  - planning: 0.142 ms
  - execution: 1.429 s
    - I/O read: 699.761 ms
    - I/O write: N/A

Shared buffers:
  - hits: 2565 (~20.00 MiB) from the buffer pool
  - reads: 858 (~6.70 MiB) from the OS file cache, including disk I/O
  - dirtied: 851 (~6.60 MiB)
  - writes: 0

This looks to impact 895 rows:

 ModifyTable on public.services  (cost=0.42..1310.07 rows=897 width=6) (actual time=716.120..716.121 rows=0 loops=1)
   Buffers: shared hit=2565 read=858 dirtied=851
   I/O Timings: read=699.761
   ->  Index Scan using index_services_on_type on public.services  (cost=0.42..1310.07 rows=897 width=6) (actual time=7.045..697.532 rows=895 loops=1)
         Index Cond: ((services.type)::text = 'AlertsService'::text)
         Buffers: shared hit=3 read=857 dirtied=15
         I/O Timings: read=692.914

I opted for a single-query post-migration here, as we don't expect a huge number of records for deletion. From a cold cache on #database-lab, we're looking at about 1.4s. Though the operation pushes the limit on what might be appropriate for a single query, I think this is appropriate considering that it's a bulk deletion and dot-com is probably the biggest table we'll be looking at. I felt this scenario was pretty borderline, so I opted to leave batching out, though it shouldn't be too hard to add if I'm off-base.

Screenshots

I've tested this code removal both with and without AlertsService records in the DB.

Integrations list Old alerts integration URL Unrelated integration URL Alerts page Alert generated by old alerts service - sys notes Alert generated by old alerts service - details Settings > Operations > Alerts
Screen_Shot_2021-01-07_at_6.04.54_PM Screen_Shot_2021-01-07_at_6.05.32_PM Screen_Shot_2021-01-07_at_6.05.22_PM Screen_Shot_2021-01-07_at_6.06.02_PM Screen_Shot_2021-01-07_at_6.08.39_PM Screen_Shot_2021-01-07_at_6.08.47_PM Screen_Shot_2021-01-07_at_6.09.27_PM

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
Edited by Mayra Cabrera

Merge request reports