Skip to content

Stage Events without end stage criteria met are not removed by ConsistencyCheckService

This is a follow-up from !117986 (comment 1361557653)

It turns out there is a bug in the ConsistencyCheckService as events don't get deleted when their associated record gets removed if the end event did not occur.

This might mean there are extra events in the DB we have to clean up.

Implementation plan

Unfortunately, we don't have a good index supporting the iteration where we include end events with null values. This means that we'll need to add a new index (async) and update the iteration logic.

  1. Add a new index to both MR and Issue stage events table: (stage_event_hash_id, group_id, end_event_timestamp, merge_request_id)
  2. Add a new order scope in Analytics::CycleAnalytics::StageEventModel (start event timestamp is missing from the order by clause and we set the nullable option for the end event timestamp)
         scope :order_by_end_event_for_consistency_check, -> (direction) do
           keyset_order(
             :end_event_timestamp => { order_expression: arel_order(arel_table[:end_event_timestamp], direction), distinct: false, nullable: direction == :asc ? :nulls_last : :nulls_first },
             issuable_id_column => { order_expression: arel_order(arel_table[issuable_id_column], direction), distinct: true }
           )
         end
    
  3. In the Analytics::CycleAnalytics::ConsistencyCheckService, remove the where.not(end_event_timestamp: nil) filter from the iterator scope.
  4. Reduce the batch size to 500, I noticed that the queries are quite heavy

Adding indexes will be the most time consuming task since these tables are partitioned. We should look into doing the index creation async: https://docs.gitlab.com/ee/development/database/adding_database_indexes.html#create-indexes-asynchronously

The issue will require 3 MRs:

  • Add async index over the weekend.
  • Create the actual index.
  • Update the iteration logic based on the diff below.

Diff for the minimal code changes (I didn't run the pipeline): changes.diff

Edited by Adam Hegyi