Skip to content

Add support for end time to oncall rotations

Sarah Yasonik requested to merge sy-add-end-date-to-rotations into master

What does this MR do?

Adds support for an end date & time for on-call rotations. Related issue: #262858 (closed)

Changes:

  • Add ends_at column & attribute for IncidentManagement::OncallRotation
  • Expose endsAt via GraphQL & accept endsAt on rotation creation
  • Restrict shifts generated for a rotation to not occur after rotation end time
  • Opt out of persisting historical shifts for ended rotations

Up migration

$ rails db:migrate
== 20210208200914 AddEndsAtToOncallRotations: migrating =======================
-- add_column(:incident_management_oncall_rotations, :ends_at, :datetime_with_timezone)
   -> 0.0098s
== 20210208200914 AddEndsAtToOncallRotations: migrated (0.0098s) ==============
DB after migration
$ bin/rails dbconsole
psql (11.9)
Type "help" for help.

gitlabhq_development=# \d incident_management_oncall_rotations
                                           Table "public.incident_management_oncall_rotations"
       Column       |           Type           | Collation | Nullable |                             Default                              
--------------------+--------------------------+-----------+----------+------------------------------------------------------------------
 id                 | bigint                   |           | not null | nextval('incident_management_oncall_rotations_id_seq'::regclass)
 created_at         | timestamp with time zone |           | not null | 
 updated_at         | timestamp with time zone |           | not null | 
 oncall_schedule_id | bigint                   |           | not null | 
 length             | integer                  |           | not null | 
 length_unit        | smallint                 |           | not null | 
 starts_at          | timestamp with time zone |           | not null | 
 name               | text                     |           | not null | 
 ends_at            | timestamp with time zone |           |          | 
Indexes:
    "incident_management_oncall_rotations_pkey" PRIMARY KEY, btree (id)
    "index_inc_mgmnt_oncall_rotations_on_oncall_schedule_id_and_id" UNIQUE, btree (oncall_schedule_id, id)
    "index_inc_mgmnt_oncall_rotations_on_oncall_schedule_id_and_name" UNIQUE, btree (oncall_schedule_id, name)
Check constraints:
    "check_5209fb5d02" CHECK (char_length(name) <= 200)
Foreign-key constraints:
    "fk_rails_256e0bc604" FOREIGN KEY (oncall_schedule_id) REFERENCES incident_management_oncall_schedules(id) ON DELETE CASCADE
Referenced by:
    TABLE "incident_management_oncall_participants" CONSTRAINT "fk_rails_032b12996a" FOREIGN KEY (oncall_rotation_id) REFERENCES incident_management_oncall_rotations(id) ON DELETE CASCADE
    TABLE "incident_management_oncall_shifts" CONSTRAINT "fk_rails_df4feb286a" FOREIGN KEY (rotation_id) REFERENCES incident_management_oncall_rotations(id) ON DELETE CASCADE

Down migration

$ rails db:rollback
== 20210208200914 AddEndsAtToOncallRotations: reverting =======================
-- remove_column(:incident_management_oncall_rotations, :ends_at, :datetime_with_timezone)
   -> 0.0041s
== 20210208200914 AddEndsAtToOncallRotations: reverted (0.0067s) ==============
DB after migration
$ bin/rails dbconsole
psql (11.9)
Type "help" for help.

gitlabhq_development=# \d incident_management_oncall_rotations
                                           Table "public.incident_management_oncall_rotations"
       Column       |           Type           | Collation | Nullable |                             Default                              
--------------------+--------------------------+-----------+----------+------------------------------------------------------------------
 id                 | bigint                   |           | not null | nextval('incident_management_oncall_rotations_id_seq'::regclass)
 created_at         | timestamp with time zone |           | not null | 
 updated_at         | timestamp with time zone |           | not null | 
 oncall_schedule_id | bigint                   |           | not null | 
 length             | integer                  |           | not null | 
 length_unit        | smallint                 |           | not null | 
 starts_at          | timestamp with time zone |           | not null | 
 name               | text                     |           | not null | 
Indexes:
    "incident_management_oncall_rotations_pkey" PRIMARY KEY, btree (id)
    "index_inc_mgmnt_oncall_rotations_on_oncall_schedule_id_and_id" UNIQUE, btree (oncall_schedule_id, id)
    "index_inc_mgmnt_oncall_rotations_on_oncall_schedule_id_and_name" UNIQUE, btree (oncall_schedule_id, name)
Check constraints:
    "check_5209fb5d02" CHECK (char_length(name) <= 200)
Foreign-key constraints:
    "fk_rails_256e0bc604" FOREIGN KEY (oncall_schedule_id) REFERENCES incident_management_oncall_schedules(id) ON DELETE CASCADE
Referenced by:
    TABLE "incident_management_oncall_participants" CONSTRAINT "fk_rails_032b12996a" FOREIGN KEY (oncall_rotation_id) REFERENCES incident_management_oncall_rotations(id) ON DELETE CASCADE
    TABLE "incident_management_oncall_shifts" CONSTRAINT "fk_rails_df4feb286a" FOREIGN KEY (rotation_id) REFERENCES incident_management_oncall_rotations(id) ON DELETE CASCADE

Query performance

Seeding local data for testing

I've taken to generating seed data in my local env via a test. For this query, I generated schedules & rotations for 1000 projects. It takes ~5minutes to run.

Total Projects: 1001
Total IncidentManagement::OncallSchedules: 3047
Total IncidentManagement::OncallRotations: 16663

The below is the test I'm running:

# frozen_string_literal: true

require 'spec_helper'
require 'activerecord-explain-analyze'

RSpec.describe Projects::Alerting::NotifyService do
  INDEX_NAME = 'temporary_rotation_start_end_index'

  before_all do
    ActiveRecord::Migration.add_index :incident_management_oncall_rotations, [:starts_at, :ends_at], name: INDEX_NAME

    200.times { create_project_with_oncall }
    @project = create_project_with_oncall
    100.times { create_project_with_oncall }

    p "#{Time.current} - Done generating projects."
  end

  after(:all) do
    ActiveRecord::Migration.remove_index :incident_management_oncall_rotations, name: INDEX_NAME
  end

  describe do
    def query(query_time)
      puts "\n\n\nROTATION IN-PROGRESS QUERY EXPLAIN\n\n"
      travel_to(query_time) do
        puts IncidentManagement::OncallRotation.in_progress.select(:id).explain(analyze: true)
      end
    end

    it 'none have started' do query(7.months.ago) end
    it 'few have started' do query(5.months.ago) end
    it 'many have started' do query(Time.current) end
    it 'many have stopped' do query(5.months.from_now) end
    it 'all have stopped' do query(1.year.from_now) end
    it 'lets me do extra queries' do
      binding.pry
    end
  end

  private

  def create_project_with_oncall
    project = create(:project)
    p "#{Time.current} - Creating project: #{project.id}"

    schedule_count = rand(1..5)
    create_list(:incident_management_oncall_schedule, schedule_count, project: project) do |schedule|
      rotation_count = rand(3..8)

      schedule.rotations = create_list(:incident_management_oncall_rotation, rotation_count, schedule: schedule) do |rotation|
        rotation.update(
          starts_at: rand(6.months.ago..1.month.from_now),
          length_unit: [:hours, :days, :weeks].sample,
          length: rand(1..12),
          ends_at: [true, false].sample ? rotation.starts_at + rand(1.month..6.months) : nil
        )
      end
    end

    p "#{Time.current}"
    p "#{Time.current} - IncidentManagement::OncallRotations: #{IncidentManagement::OncallRotation.count}"
    p "#{Time.current} - IncidentManagement::OncallSchedules: #{IncidentManagement::OncallSchedule.count}"
    project
  end
end

And here's the trimmed output:

Without index:

ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:46:55.452691' AND (ends_at > '2021-02-17 00:46:55.452691' OR ends_at IS NULL)) /*application:test,correlation_id:df5fb8f7f827bda964faa449e151bac2*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..270.51 rows=1044 width=8) (actual time=0.017..1.912 rows=4242 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-02-17 00:46:55.452691+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:46:55.452691+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 646
  Buffers: shared hit=131
Planning Time: 0.045 ms
Execution Time: 2.190 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:46:55.484088' AND (ends_at > '2021-02-17 00:46:55.484088' OR ends_at IS NULL)) /*application:test,correlation_id:7405c8678590dc8730dd270883648959*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..270.51 rows=1044 width=8) (actual time=0.019..2.069 rows=4242 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-02-17 00:46:55.484088+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:46:55.484088+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 646
  Buffers: shared hit=131
Planning Time: 0.047 ms
Execution Time: 2.373 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:46:55.514623' AND (ends_at > '2021-02-17 00:46:55.514623' OR ends_at IS NULL)) /*application:test,correlation_id:76a1b2bcce252048cbe984c25617d061*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..270.51 rows=1044 width=8) (actual time=0.020..2.152 rows=4242 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-02-17 00:46:55.514623+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:46:55.514623+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 646
  Buffers: shared hit=131
Planning Time: 0.047 ms
Execution Time: 2.455 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:46:55.545763' AND (ends_at > '2021-02-17 00:46:55.545763' OR ends_at IS NULL)) /*application:test,correlation_id:072468a4fcb67b2384203bf64d05435f*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..270.51 rows=1044 width=8) (actual time=0.017..1.936 rows=4242 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-02-17 00:46:55.545763+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:46:55.545763+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 646
  Buffers: shared hit=131
Planning Time: 0.044 ms
Execution Time: 2.211 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:46:55.575585' AND (ends_at > '2021-02-17 00:46:55.575585' OR ends_at IS NULL)) /*application:test,correlation_id:9413d980152d9a7b156c1abc166f76ae*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..270.51 rows=1044 width=8) (actual time=0.020..2.142 rows=4242 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-02-17 00:46:55.575585+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:46:55.575585+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 646
  Buffers: shared hit=131
Planning Time: 0.050 ms
Execution Time: 2.436 ms

With index:

ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:52:57.726423' AND (ends_at > '2021-02-17 00:52:57.726423' OR ends_at IS NULL)) /*application:test,correlation_id:48bde1995df64f45b47b5556f4db7790*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..217.25 rows=1060 width=8) (actual time=0.041..3.488 rows=4291 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-02-17 00:52:57.726423+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:52:57.726423+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=4553
Planning Time: 0.057 ms
Execution Time: 3.758 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:52:57.761020' AND (ends_at > '2021-02-17 00:52:57.761020' OR ends_at IS NULL)) /*application:test,correlation_id:1f6b4dece1b614886970d02a1ab87677*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..217.25 rows=1060 width=8) (actual time=0.015..3.432 rows=4291 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-02-17 00:52:57.76102+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:52:57.76102+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=4553
Planning Time: 0.057 ms
Execution Time: 3.700 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:52:57.793154' AND (ends_at > '2021-02-17 00:52:57.793154' OR ends_at IS NULL)) /*application:test,correlation_id:3c198d1d36c3261e756ee1dc8f91408b*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..217.25 rows=1060 width=8) (actual time=0.017..3.568 rows=4291 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-02-17 00:52:57.793154+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:52:57.793154+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=4553
Planning Time: 0.063 ms
Execution Time: 3.849 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:52:57.826153' AND (ends_at > '2021-02-17 00:52:57.826153' OR ends_at IS NULL)) /*application:test,correlation_id:5bd5f0cf4a168e8af2406aacea8b8350*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..217.25 rows=1060 width=8) (actual time=0.017..3.601 rows=4291 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-02-17 00:52:57.826153+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:52:57.826153+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=4553
Planning Time: 0.058 ms
Execution Time: 3.873 ms
.


ROTATION IN-PROGRESS QUERY EXPLAIN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 00:52:57.859437' AND (ends_at > '2021-02-17 00:52:57.859437' OR ends_at IS NULL)) /*application:test,correlation_id:2a9185dd23dcc69a6bbce42171efd106*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..217.25 rows=1060 width=8) (actual time=0.015..3.526 rows=4291 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-02-17 00:52:57.859437+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-02-17 00:52:57.859437+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=4553
Planning Time: 0.057 ms
Execution Time: 3.792 ms

It's slower? Why?

Trying again with a larger batch (1000 projects; 5min) w/o index:

IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2020-07-17 02:04:49' AND (ends_at > '2020-07-17 02:04:49' OR ends_at IS NULL)) /*application:test,correlation_id:f1cb191d7b1fc1efc0f1f9a328346a63*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..918.92 rows=3546 width=8) (actual time=8.463..8.464 rows=0 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2020-07-17 02:04:49+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2020-07-17 02:04:49+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 16663
  Buffers: shared hit=445
Planning Time: 0.038 ms
Execution Time: 8.474 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2020-09-17 02:04:49' AND (ends_at > '2020-09-17 02:04:49' OR ends_at IS NULL)) /*application:test,correlation_id:fbce452a1f8f2194dadd85220bd3b508*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..918.92 rows=3546 width=8) (actual time=0.023..6.416 rows=2488 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2020-09-17 02:04:49+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2020-09-17 02:04:49+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 14175
  Buffers: shared hit=445
Planning Time: 0.063 ms
Execution Time: 6.590 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 02:04:49' AND (ends_at > '2021-02-17 02:04:49' OR ends_at IS NULL)) /*application:test,correlation_id:98a5fa627bbd32d5fee0f93f8c34323d*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..918.92 rows=3546 width=8) (actual time=0.017..6.250 rows=14468 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-02-17 02:04:49+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-02-17 02:04:49+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 2195
  Buffers: shared hit=445
Planning Time: 0.041 ms
Execution Time: 7.096 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-07-17 02:04:49' AND (ends_at > '2021-07-17 02:04:49' OR ends_at IS NULL)) /*application:test,correlation_id:5f6d97ddf4cdd1dd76c5d40c917b301c*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..918.92 rows=3546 width=8) (actual time=0.017..6.220 rows=9859 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2021-07-17 02:04:49+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2021-07-17 02:04:49+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 6804
  Buffers: shared hit=445
Planning Time: 0.043 ms
Execution Time: 6.800 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2022-02-17 02:04:49' AND (ends_at > '2022-02-17 02:04:49' OR ends_at IS NULL)) /*application:test,correlation_id:2c8e8ceb628e99b65501be8afc3ce5d0*/
Seq Scan on public.incident_management_oncall_rotations  (cost=0.00..918.92 rows=3546 width=8) (actual time=0.018..6.379 rows=8244 loops=1)
  Output: id
  Filter: ((incident_management_oncall_rotations.starts_at < '2022-02-17 02:04:49+00'::timestamp with time zone) AND ((incident_management_oncall_rotations.ends_at > '2022-02-17 02:04:49+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL)))
  Rows Removed by Filter: 8419
  Buffers: shared hit=445
Planning Time: 0.042 ms
Execution Time: 6.864 ms

Trying again with a larger batch (1000 projects; 5min) w/ index:

IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2020-07-17 01:57:54' AND (ends_at > '2020-07-17 01:57:54' OR ends_at IS NULL)) /*application:test,correlation_id:1095f96aad923dcbe01aaa1359b00994*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..725.87 rows=3562 width=8) (actual time=0.003..0.003 rows=0 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2020-07-17 01:57:54+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2020-07-17 01:57:54+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=2
Planning Time: 0.041 ms
Execution Time: 0.011 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2020-09-17 01:57:54' AND (ends_at > '2020-09-17 01:57:54' OR ends_at IS NULL)) /*application:test,correlation_id:0078d53169366f557f35d63d8cf083cd*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..725.87 rows=3562 width=8) (actual time=0.014..1.849 rows=2421 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2020-09-17 01:57:54+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2020-09-17 01:57:54+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=2431
Planning Time: 0.063 ms
Execution Time: 2.029 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-02-17 01:57:54' AND (ends_at > '2021-02-17 01:57:54' OR ends_at IS NULL)) /*application:test,correlation_id:304f5104cc9700a26a8e373800ff802d*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..725.87 rows=3562 width=8) (actual time=0.013..11.981 rows=14553 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-02-17 01:57:54+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-02-17 01:57:54+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Buffers: shared hit=15491
Planning Time: 0.054 ms
Execution Time: 12.835 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2021-07-17 01:57:54' AND (ends_at > '2021-07-17 01:57:54' OR ends_at IS NULL)) /*application:test,correlation_id:c8dd877f748b156ce103e529a254cdad*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..725.87 rows=3562 width=8) (actual time=0.014..12.758 rows=9984 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2021-07-17 01:57:54+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2021-07-17 01:57:54+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Rows Removed by Filter: 6756
  Buffers: shared hit=17680
Planning Time: 0.056 ms
Execution Time: 13.346 ms
.


IN-PROGRESS QUERY PLAN

EXPLAIN for: SELECT "incident_management_oncall_rotations"."id" FROM "incident_management_oncall_rotations" WHERE (starts_at < '2022-02-17 01:57:54' AND (ends_at > '2022-02-17 01:57:54' OR ends_at IS NULL)) /*application:test,correlation_id:4afee255d59c534824d0899151bf6b61*/
Index Scan using temporary_rotation_start_end_index on public.incident_management_oncall_rotations  (cost=0.29..725.87 rows=3562 width=8) (actual time=0.014..13.120 rows=8379 loops=1)
  Output: id
  Index Cond: (incident_management_oncall_rotations.starts_at < '2022-02-17 01:57:54+00'::timestamp with time zone)
  Filter: ((incident_management_oncall_rotations.ends_at > '2022-02-17 01:57:54+00'::timestamp with time zone) OR (incident_management_oncall_rotations.ends_at IS NULL))
  Rows Removed by Filter: 8361
  Buffers: shared hit=17680
Planning Time: 0.059 ms
Execution Time: 13.624 ms
Summary

Because we expect many or most rotations to be selected as 'in-progress', the sequential scan seems to be the best thing for the common case. The more finished/future rotations we have, the more value an index has.

As this is also not a user-facing query, the burden of the less common case will be bore by us rather than a user directly. If this table does grow in the number of not-in-progress rotations, that could be difficult to identify because we won't interact with the query in normal interactions. We'd have to check in on it. I'm thinking that's the best thing. Keep it sequential for now & create an issue for 6months out to see actual usage. New issue created: #321870 (closed)

SQL
SELECT "incident_management_oncall_rotations"."id" 
FROM "incident_management_oncall_rotations" 
WHERE (
  starts_at < '2022-02-17 01:57:54' 
  AND (ends_at > '2022-02-17 01:57:54' OR ends_at IS NULL)
) 
Query plans
Use-case With index on rotations.[starts_at, :ends_at] Without index
No rotations have started Planning Time: 0.041 ms
Execution Time: 0.011 ms
https://explain.depesz.com/s/BQMb
Planning Time: 0.038 ms
Execution Time: 8.474 ms
https://explain.depesz.com/s/Utru
A few rotations are in progress Planning Time: 0.063 ms
Execution Time: 2.029 ms
https://explain.depesz.com/s/PHn0
Planning Time: 0.063 ms
Execution Time: 6.590 ms
https://explain.depesz.com/s/4tKJ
Many rotations are in progress Planning Time: 0.054 ms
Execution Time: 12.835 ms
https://explain.depesz.com/s/2HSU
Planning Time: 0.041 ms
Execution Time: 7.096 ms
https://explain.depesz.com/s/DyIN
Many rotations have ended Planning Time: 0.056 ms
Execution Time: 13.346 ms
https://explain.depesz.com/s/SaCM
Planning Time: 0.043 ms
Execution Time: 6.800 ms
https://explain.depesz.com/s/huyi
All rotations with endings have ended Planning Time: 0.059 ms
Execution Time: 13.624 ms
https://explain.depesz.com/s/8o60
Planning Time: 0.042 ms
Execution Time: 6.864 ms
https://explain.depesz.com/s/4o8H

GraphQL

Query:

mutation createRotation($createRotationInput: OncallRotationCreateInput!) {
  oncallRotationCreate(input: $createRotationInput) {
    oncallRotation {
      id
      startsAt
      endsAt
    }
    errors
  }
}

With ends_at: null:

Input:

{
  "createRotationInput": {
    "projectPath": "root/autodevops",
    "scheduleIid": "74",
    "name": "Test for EndsAt API call - empty ends_at",
    "startsAt": {
      "date": "2020-11-09",
      "time": "14:13"
    },
    "endsAt": null,
    "rotationLength": {
      "length": 5,
      "unit": "HOURS"
    },
    "participants": [
      {
        "username": "root",
        "colorWeight": "WEIGHT_500",
        "colorPalette": "BLUE"
      }
    ]
  }
}

Output:

{
  "data": {
    "oncallRotationCreate": {
      "oncallRotation": {
        "id": "gid://gitlab/IncidentManagement::OncallRotation/79",
        "startsAt": "2020-11-09T19:13:00Z",
        "endsAt": null
      },
      "errors": []
    }
  }
}

With valid ends_at:

Input:

{
  "createRotationInput": {
    "projectPath": "root/autodevops",
    "scheduleIid": "74",
    "name": "Test for EndsAt API call - valid ends_at",
    "startsAt": {
      "date": "2020-11-09",
      "time": "14:13"
    },
    "endsAt": {
      "date": "2020-11-14",
      "time": "14:13"
    },
    "rotationLength": {
      "length": 5,
      "unit": "HOURS"
    },
    "participants": [
      {
        "username": "root",
        "colorWeight": "WEIGHT_500",
        "colorPalette": "BLUE"
      }
    ]
  }
}

Output:

{
  "data": {
    "oncallRotationCreate": {
      "oncallRotation": {
        "id": "gid://gitlab/IncidentManagement::OncallRotation/80",
        "startsAt": "2020-11-09T19:13:00Z",
        "endsAt": "2020-11-14T19:13:00Z"
      },
      "errors": []
    }
  }
}

With invalid ends_at:

Input:

{
  "createRotationInput": {
    "projectPath": "root/autodevops",
    "scheduleIid": "74",
    "name": "Test for EndsAt API call - invalid ends_at",
    "startsAt": {
      "date": "2020-12-09",
      "time": "14:13"
    },
    "endsAt":  {
      "date": "2020-11-14",
      "time": "14:13"
    },
    "rotationLength": {
      "length": 5,
      "unit": "HOURS"
    },
    "participants": [
      {
        "username": "root",
        "colorWeight": "WEIGHT_500",
        "colorPalette": "BLUE"
      }
    ]
  }
}

Output:

{
  "data": {
    "oncallRotationCreate": {
      "oncallRotation": null,
      "errors": [
        "Ends at must be after start"
      ]
    }
  }
}

Related/Conflicting MR

!52998 (merged) has some overlapping changes. Most changes should be compatible, but whichever merges second will need some small modifications in ee/lib/incident_management/oncall_shift_generator.rb.

Anticipated resolution for conflicts:

diff --git a/ee/lib/incident_management/oncall_shift_generator.rb b/ee/lib/incident_management/oncall_shift_generator.rb
def shift_cycle_for(elapsed_shift_cycle_count, shift_cycle_starts_at)
  participant = participants[participant_rank(elapsed_shift_cycle_count)]

  if rotation.has_shift_active_period?
    # the number of shifts we expect to be included in the
    # shift_cycle. 1.week is the same as 7.days.
    expected_shift_count = rotation.shifts_per_cycle
    (0..expected_shift_count - 1).map do |shift_count|
      # we know the start/end time of the active period,
      # so the date is dependent on the cycle start time
      # and how many days have elapsed in the cycle.
      # EX) shift_cycle_starts_at = Monday @ 8am
      #     active_period_start = 8am
      #     active_period_end = 5pm
      #     expected_shift_count = 14          -> pretend it's a 2-week rotation
      #     shift_count = 2                    -> we're calculating the shift for the 3rd day
      # starts_at = Monday 00:00:00 + 8.hours + 2.days => Thursday 08:00:00

      starts_at, ends_at = rotation.active_period.for_date(shift_cycle_starts_at + shift_count.days)
 
-     shift_for(participant, starts_at, ends_at)
+     shift_for(participant, starts_at, limit_end_time(ends_at))
    end
  else
-   shift_for(participant, shift_cycle_starts_at, shift_cycle_starts_at + shift_cycle_duration)
+   shift_cycle_ends_at = limit_end_time(shift_cycle_starts_at + shift_cycle_duration)
+
+   shift_for(participant, shift_cycle_starts_at, shift_cycle_ends_at)
  end
end

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 Sarah Yasonik

Merge request reports