Add 2 new sort options to runner API: CREATED_ASC and CONTACTED_DESC (!62310) · Merge requests · GitLab.org / GitLab

Pedro Pombeiro requested to merge pedropombeiro/331600/add-sort-options-to-runner-api into master May 21, 2021

What does this MR do?

This MR adds 2 additional sort options to the existing runners query: CONTACTED_DESC and CREATED_ASC. It adds the options to the RunnersFinder class, matching scopes to the Runner model, and tests to the RunnersResolver spec.

Example of a query:

query getRunners {
  runners(sort: CONTACTED_DESC) {
    edges {
      node {
        # ... RunnerType
        id
        shortSha
        description
        contactedAt
        status
        tagList
        # ... RunnerType
      }
    }
    pageInfo{
      hasNextPage
      hasPreviousPage
      startCursor
      endCursor
    }
  }
}

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

I have included a changelog entry, or it's not needed. (Does this MR need a changelog?)
I have added/updated documentation, or it's not needed. (Is documentation required?)
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?)
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?)
I have self-reviewed this MR per code review guidelines.
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines)
I have followed the style guides.

Availability and Testing

I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.)
I have tested this MR in all supported browsers, or it's not needed.
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

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

Database

Migrations

Migrate up

$ rails db:migrate RAILS_ENV=development
== 20210601125410 AddRunnersCreatedAtIndex: migrating =========================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, [:created_at, :id], {:order=>{:id=>:desc}, :name=>"index_ci_runners_on_created_at_and_id_desc", :algorithm=>:concurrently})
   -> 0.0043s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- add_index(:ci_runners, [:created_at, :id], {:order=>{:id=>:desc}, :name=>"index_ci_runners_on_created_at_and_id_desc", :algorithm=>:concurrently})
   -> 0.0081s
-- execute("RESET ALL")
   -> 0.0005s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, [:created_at, :id], {:order=>{:created_at=>:desc, :id=>:desc}, :name=>"index_ci_runners_on_created_at_desc_and_id_desc", :algorithm=>:concurrently})
   -> 0.0024s
-- add_index(:ci_runners, [:created_at, :id], {:order=>{:created_at=>:desc, :id=>:desc}, :name=>"index_ci_runners_on_created_at_desc_and_id_desc", :algorithm=>:concurrently})
   -> 0.0024s
== 20210601125410 AddRunnersCreatedAtIndex: migrated (0.0197s) ================

== 20210601133459 ReplaceRunnersContactedAtIndex: migrating ===================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, [:contacted_at, :id], {:order=>{:id=>:desc}, :name=>"index_ci_runners_on_contacted_at_and_id_desc", :using=>"btree", :algorithm=>:concurrently})
   -> 0.0026s
-- add_index(:ci_runners, [:contacted_at, :id], {:order=>{:id=>:desc}, :name=>"index_ci_runners_on_contacted_at_and_id_desc", :using=>"btree", :algorithm=>:concurrently})
   -> 0.0019s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, [:contacted_at, :id], {:order=>{:contacted_at=>:desc, :id=>:desc}, :name=>"index_ci_runners_on_contacted_at_desc_and_id_desc", :using=>"btree", :algorithm=>:concurrently})
   -> 0.0025s
-- add_index(:ci_runners, [:contacted_at, :id], {:order=>{:contacted_at=>:desc, :id=>:desc}, :name=>"index_ci_runners_on_contacted_at_desc_and_id_desc", :using=>"btree", :algorithm=>:concurrently})
   -> 0.0019s
-- transaction_open?()
   -> 0.0000s
-- indexes(:ci_runners)
   -> 0.0026s
-- remove_index(:ci_runners, {:algorithm=>:concurrently, :name=>"index_ci_runners_on_contacted_at"})
   -> 0.0038s
== 20210601133459 ReplaceRunnersContactedAtIndex: migrated (0.0176s) ==========

rails db:migrate RAILS_ENV=development  8.23s user 6.18s system 82% cpu 17.548 total

Migrate down

$ rails db:migrate:down VERSION=20210601125410 && rails db:migrate:down VERSION=20210601133459
== 20210601125410 AddRunnersCreatedAtIndex: reverting =========================
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, [:created_at, :id], {:order=>{:id=>:desc}, :name=>"index_ci_runners_on_created_at_and_id_desc", :algorithm=>:concurrently})
   -> 0.0042s
-- execute("SET statement_timeout TO 0")
   -> 0.0006s
-- remove_index(:ci_runners, {:order=>{:id=>:desc}, :name=>"index_ci_runners_on_created_at_and_id_desc", :algorithm=>:concurrently, :column=>[:created_at, :id]})
   -> 0.0051s
-- execute("RESET ALL")
   -> 0.0006s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, [:created_at, :id], {:order=>{:created_at=>:desc, :id=>:desc}, :name=>"index_ci_runners_on_created_at_desc_and_id_desc", :algorithm=>:concurrently})
   -> 0.0025s
-- remove_index(:ci_runners, {:order=>{:created_at=>:desc, :id=>:desc}, :name=>"index_ci_runners_on_created_at_desc_and_id_desc", :algorithm=>:concurrently, :column=>[:created_at, :id]})
   -> 0.0036s
== 20210601125410 AddRunnersCreatedAtIndex: reverted (0.0182s) ================

rails db:migrate:down VERSION=20210601125410  8.10s user 6.04s system 81% cpu 17.388 total
== 20210601133459 ReplaceRunnersContactedAtIndex: reverting ===================
-- transaction_open?()
   -> 0.0000s
-- indexes(:ci_runners)
   -> 0.0034s
-- execute("SET statement_timeout TO 0")
   -> 0.0004s
-- remove_index(:ci_runners, {:algorithm=>:concurrently, :name=>"index_ci_runners_on_contacted_at_and_id_desc"})
   -> 0.0025s
-- execute("RESET ALL")
   -> 0.0005s
-- transaction_open?()
   -> 0.0000s
-- indexes(:ci_runners)
   -> 0.0019s
-- remove_index(:ci_runners, {:algorithm=>:concurrently, :name=>"index_ci_runners_on_contacted_at_desc_and_id_desc"})
   -> 0.0013s
-- transaction_open?()
   -> 0.0000s
-- index_exists?(:ci_runners, :contacted_at, {:name=>"index_ci_runners_on_contacted_at", :using=>"btree", :algorithm=>:concurrently})
   -> 0.0015s
-- add_index(:ci_runners, :contacted_at, {:name=>"index_ci_runners_on_contacted_at", :using=>"btree", :algorithm=>:concurrently})
   -> 0.0023s
== 20210601133459 ReplaceRunnersContactedAtIndex: reverted (0.0159s) ==========

rails db:migrate:down VERSION=20210601133459  7.98s user 5.75s system 85% cpu 16.070 total

Database plans

runners(sort: CREATED_ASC)

SELECT
    "ci_runners".*
FROM
    "ci_runners"
ORDER BY
    "ci_runners"."created_at" ASC,
    "ci_runners"."id" DESC
LIMIT 100

Limit  (cost=358852.62..358864.29 rows=100 width=231)
  ->  Gather Merge  (cost=358852.62..521922.00 rows=1397640 width=231)
        Workers Planned: 2
        ->  Sort  (cost=357852.59..359599.64 rows=698820 width=231)
              Sort Key: created_at, id DESC
              ->  Parallel Seq Scan on ci_runners  (cost=0.00..331144.20 rows=698820 width=231)

Closes #331600 (closed)

Edited Jun 07, 2021 by Pedro Pombeiro

Add 2 new sort options to runner API: CREATED_ASC and CONTACTED_DESC