From 131ccd55658a63bfbf137de8bef5dc33a7086f1d Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Wed, 24 Jul 2024 14:07:48 -0400 Subject: [PATCH 1/7] Update pipeline newest_first updates class method that queries newest pipelines that had causing extra subqueries EE: false --- app/models/ci/pipeline.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f37fa2edcf81c3fb..52be9c7fd49361b0 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -474,8 +474,7 @@ class Pipeline < Ci::ApplicationRecord # ref - The name (or names) of the branch(es)/tag(s) to limit the list of # pipelines to. # sha - The commit SHA (or multiple SHAs) to limit the list of pipelines to. - # limit - This limits a backlog search, default to 100. - def self.newest_first(ref: nil, sha: nil, limit: 100) + def self.newest_first(ref: nil, sha: nil, limit: nil) relation = order(id: :desc) relation = relation.where(ref: ref) if ref relation = relation.where(sha: sha) if sha -- GitLab From 9b1d4461858cc6cadfc96cb20fbcf5df32f607e4 Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Wed, 24 Jul 2024 14:11:13 -0400 Subject: [PATCH 2/7] Update method calls that need limit updates 2 method calls that rely on previous default limit EE: false --- app/models/project.rb | 2 +- app/services/ci/create_commit_status_service.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 051b5d9e2ecf049f..0f7ea8b3c5c1603b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1425,7 +1425,7 @@ def latest_pipelines(ref = default_branch, sha = nil) sha ||= commit(ref)&.sha return ci_pipelines.none unless sha - ci_pipelines.newest_first(ref: ref, sha: sha) + ci_pipelines.newest_first(ref: ref, sha: sha, limit: 100) end def latest_pipeline(ref = default_branch, sha = nil) diff --git a/app/services/ci/create_commit_status_service.rb b/app/services/ci/create_commit_status_service.rb index 181140808a16331a..00c9dc1cd2102108 100644 --- a/app/services/ci/create_commit_status_service.rb +++ b/app/services/ci/create_commit_status_service.rb @@ -60,7 +60,7 @@ def commit strong_memoize_attr :commit def first_matching_pipeline - pipelines = project.ci_pipelines.newest_first(sha: sha) + pipelines = project.ci_pipelines.newest_first(sha: sha, limit: 100) pipelines = pipelines.for_ref(params[:ref]) if params[:ref] pipelines = pipelines.id_in(params[:pipeline_id]) if params[:pipeline_id] pipelines.first -- GitLab From 5a247fb99baaaa22aad933eb3cff73f547fcffec Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Wed, 24 Jul 2024 14:35:41 -0400 Subject: [PATCH 3/7] Update Project lates pipelines query Update latest pipeline query to bypass limit subquery EE:false --- app/models/project.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 0f7ea8b3c5c1603b..04c9a273d8ed4e89 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1420,16 +1420,16 @@ def latest_successful_build_for_ref!(job_name, ref = default_branch) latest_successful_build_for_ref(job_name, ref) || raise(ActiveRecord::RecordNotFound, "Couldn't find job #{job_name}") end - def latest_pipelines(ref = default_branch, sha = nil) + def latest_pipelines(ref = default_branch, sha = nil, limit = 100) ref = ref.presence || default_branch sha ||= commit(ref)&.sha return ci_pipelines.none unless sha - ci_pipelines.newest_first(ref: ref, sha: sha, limit: 100) + ci_pipelines.newest_first(ref: ref, sha: sha, limit: limit) end def latest_pipeline(ref = default_branch, sha = nil) - latest_pipelines(ref, sha).take + latest_pipelines(ref, sha, nil).take end def merge_base_commit(first_commit_id, second_commit_id) -- GitLab From b97932c4d9c7354b2c5fb5d027ac650916406d27 Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Fri, 26 Jul 2024 11:17:21 -0400 Subject: [PATCH 4/7] Add comment explaining changes Updates comment to reflect changes being implemented EE: false --- app/models/ci/pipeline.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 52be9c7fd49361b0..9e7ac237cdb1392f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -474,6 +474,11 @@ class Pipeline < Ci::ApplicationRecord # ref - The name (or names) of the branch(es)/tag(s) to limit the list of # pipelines to. # sha - The commit SHA (or multiple SHAs) to limit the list of pipelines to. + # limit - By default there is no limit set for this query. Unless chaining + # with another limit setting method such as #take or #pick ensure an explicit + # limit is passed. + # Conversely, passing an explicit limit when also chaining with another limit + # setting method such as #take or #pick will cause unnecessary subqueries. def self.newest_first(ref: nil, sha: nil, limit: nil) relation = order(id: :desc) relation = relation.where(ref: ref) if ref -- GitLab From 2236673edb375382320284339ebb3b95ed3852ce Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Fri, 26 Jul 2024 14:18:01 -0400 Subject: [PATCH 5/7] Update limit comment EE: false --- app/models/ci/pipeline.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 9e7ac237cdb1392f..d8bf8eb1203c6967 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -474,11 +474,8 @@ class Pipeline < Ci::ApplicationRecord # ref - The name (or names) of the branch(es)/tag(s) to limit the list of # pipelines to. # sha - The commit SHA (or multiple SHAs) to limit the list of pipelines to. - # limit - By default there is no limit set for this query. Unless chaining - # with another limit setting method such as #take or #pick ensure an explicit - # limit is passed. - # Conversely, passing an explicit limit when also chaining with another limit - # setting method such as #take or #pick will cause unnecessary subqueries. + # limit - Number of pipelines to return. Chaining with sampling methods (#pick, #take) + # will cause unnecessary subqueries. def self.newest_first(ref: nil, sha: nil, limit: nil) relation = order(id: :desc) relation = relation.where(ref: ref) if ref -- GitLab From 49a49f8610350a65e8a5caa394c847d29c5b1078 Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Mon, 29 Jul 2024 10:17:54 -0400 Subject: [PATCH 6/7] Update to keywork arguments Update latest_pipelines to use keywork args --- app/controllers/projects/pipelines_controller.rb | 2 +- app/models/project.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 96e57a5eed7b58d0..9e35403524639aa2 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -284,7 +284,7 @@ def pipeline pipelines = if find_latest_pipeline? - project.latest_pipelines(params['ref']) + project.latest_pipelines(ref: params['ref'], limit: 100) else project.all_pipelines.id_in(params[:id]) end diff --git a/app/models/project.rb b/app/models/project.rb index 04c9a273d8ed4e89..f9286cbf72555adb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1420,7 +1420,7 @@ def latest_successful_build_for_ref!(job_name, ref = default_branch) latest_successful_build_for_ref(job_name, ref) || raise(ActiveRecord::RecordNotFound, "Couldn't find job #{job_name}") end - def latest_pipelines(ref = default_branch, sha = nil, limit = 100) + def latest_pipelines(ref: default_branch, sha: nil, limit: nil) ref = ref.presence || default_branch sha ||= commit(ref)&.sha return ci_pipelines.none unless sha @@ -1429,7 +1429,7 @@ def latest_pipelines(ref = default_branch, sha = nil, limit = 100) end def latest_pipeline(ref = default_branch, sha = nil) - latest_pipelines(ref, sha, nil).take + latest_pipelines(ref, sha).take end def merge_base_commit(first_commit_id, second_commit_id) -- GitLab From 080ef681f4466f9d50039c471feb3ba0dea25787 Mon Sep 17 00:00:00 2001 From: Vlad Wolanyk <vwolanyk@gitlab.com> Date: Mon, 29 Jul 2024 11:21:25 -0400 Subject: [PATCH 7/7] Use kwargs in latest_pipeline method --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index f9286cbf72555adb..70a6a34cf8399c6c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1429,7 +1429,7 @@ def latest_pipelines(ref: default_branch, sha: nil, limit: nil) end def latest_pipeline(ref = default_branch, sha = nil) - latest_pipelines(ref, sha).take + latest_pipelines(ref: ref, sha: sha).take end def merge_base_commit(first_commit_id, second_commit_id) -- GitLab