summaryrefslogtreecommitdiff
path: root/spec
Commit message (Collapse)AuthorAgeFilesLines
* Removed update methodissuable-todo-improvementsPhil Hughes2016-06-171-86/+80
| | | | | Re-structured controller spec Renamed issuable param to issuable_id
* Added todo controller tests for merge requestsPhil Hughes2016-06-171-37/+90
|
* Correctly checks if user is logged in when adding todoPhil Hughes2016-06-171-0/+13
|
* Improved manual todosPhil Hughes2016-06-172-0/+52
| | | | Based on feedback from !4502
* Merge branch 'rs-devise-emails' into 'master' Robert Speicher2016-06-171-2/+21
|\ | | | | | | | | | | | | Customize all Devise mails Continuing from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3354 See merge request !4297
| * Add previews for all customized Devise emailsRobert Speicher2016-06-161-2/+21
| |
* | Merge remote-tracking branch 'dev/master'Robert Speicher2016-06-161-0/+7
|\ \
| * \ Merge branch 'confidential-issue-notes' into 'master' Robert Speicher2016-06-161-0/+7
| |\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Project members with guest role can't access notes on confidential issues Related issues: - https://gitlab.com/gitlab-org/gitlab-ce/issues/18535 - https://gitlab.com/gitlab-org/gitlab-ce/issues/14787 See merge request !1971
| | * | Project members with guest role can't access notes on confidential issuesDouglas Barbosa Alexandre2016-06-141-0/+7
| | | |
* | | | Merge branch 'remove_jiraissue' into 'master' Robert Speicher2016-06-175-37/+10
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove JiraIssue model and replace references with ExternalIssue This MR Removes unused JiraIssue class and replaces references with ExternalIssue Closes #18203 See merge request !4659
| * | | | Remove JiraIssue model and replace references with ExternalIssueIlan Shamir2016-06-165-37/+10
| | | | |
* | | | | Merge branch 'fix-project-find-with-namespace-order' into 'master' Robert Speicher2016-06-171-1/+17
|\ \ \ \ \ | |_|_|_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixed ordering in Project.find_with_namespace This MR fixes the ordering of `Project.find_with_namespace` to ensure that it returns rows that match literally first. Closes #18603 See merge request !4682
| * | | | Fixed ordering in Project.find_with_namespacefix-project-find-with-namespace-orderYorick Peterse2016-06-161-1/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This ensures that Project.find_with_namespace returns a row matching literally as the first value, instead of returning a random value. The ordering here is _only_ applied to Project.find_with_namespace and _not_ Project.where_paths_in as currently there's no code that requires Project.where_paths_in to return rows in a certain order. Since this method also returns all rows that match there's no real harm in not setting a specific order either. Another reason is that generating all the "WHEN" arms for multiple values in Project.where_paths_in becomes really messy. On MySQL we have to use the "BINARY" operator to turn a "WHERE" into a case-sensitive WHERE as otherwise MySQL may still end up returning rows in an unpredictable order. Fixes gitlab-org/gitlab-ce#18603
* | | | | Merge branch 'banzai-issue-filter-queries' into 'master' Douwe Maan2016-06-163-4/+60
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reduce SQL query counts in IssueReferenceFilter ## What does this MR do? This MR adds a preparation phase for reference filters that allows them to prepare/create data structures used while iterating over HTML nodes. In this particular case the preparation phase is used for issue references to greatly cut down the amount of queries executed to get projects/issues for Markdown references. ## Are there points in the code the reviewer needs to double check? No. ## Why was this MR needed? Rendering Markdown containing issue references would run at most two queries for every issue reference: one to get the project and one to get the issue from said project. When rendering Markdown with lots of issue references this would result in _a lot_ of queries being executed. ## What are the relevant issue numbers? #18042 See merge request !4410
| * | | | | Reduce queries in IssueReferenceFilterbanzai-issue-filter-queriesYorick Peterse2016-06-163-4/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reduces the number of queries executed in IssueReferenceFilter by retrieving the various projects/issues that may be referenced in batches _before_ iterating over all the HTML nodes. A chunk of the logic resides in AbstractReferenceFilter so it can be re-used by other filters in the future.
* | | | | | Merge branch '13525-sane-defaults-for-merge-request-js-class-constructor' ↵Jacob Schatz2016-06-161-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | into 'master' Avoid a TypeError when initializing MergeRequest JS class with no arg ## What does this MR do? Avoid a TypeError when initializing MergeRequest JS class with no arg. ## Are there points in the code the reviewer needs to double check? No. ## Why was this MR needed? Without this sane default you would get the following error when you tried to instantiate a new MergeRequest object with no argument (i.e. `new MergeRequest();`): TypeError: undefined is not an object (evaluating 'this.opts.action') ## What are the relevant issue numbers? Fixes #13525. ## Does this MR meet the acceptance criteria? - [x] No CHANGELOG since it's a trivial internal change - [x] Tests - [x] Added for this feature/bug - [ ] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4667
| * | | | | | Avoid a TypeError when initializing MergeRequest JS class with no arg13525-sane-defaults-for-merge-request-js-class-constructorRémy Coutable2016-06-151-1/+1
| | |_|_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Without this sane default you would get the following error when you tried to instantiate a new MergeRequest object with no argument (i.e. `new MergeRequest();`): TypeError: undefined is not an object (evaluating 'this.opts.action') Signed-off-by: Rémy Coutable <remy@rymai.me>
* | | | | | Merge branch 'template_dropdown' into 'master' Jacob Schatz2016-06-163-5/+22
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Implements TemplateDropdown class to create custom template dropdowns ## What does this MR do? Refactorize template dropdowns. This MR creates a base TemplateSelector class so it can be reused for multiple types of templates. ## Does this MR meet the acceptance criteria? - [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - [ ] Tests - [ ] Added for this feature/bug - [x] All builds are passing - [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4697
| * | | | | | Implements TemplateDropdown class to create custom template dropdownstemplate_dropdownAlfredo Sumaran2016-06-163-5/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Also License dropdown has been ported to use our GL dropdown instead of Select2. Fixes tests to make it work with current implementation
* | | | | | | Merge branch '18582-banzai-filter-external-link-filter' into 'master' Yorick Peterse2016-06-161-11/+23
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Banzai::Filter::ExternalLinkFilter use XPath See merge request !4702
| * | | | | | | Banzai::Filter::ExternalLinkFilter use XPath18582-banzai-filter-external-link-filterPaco Guzman2016-06-161-11/+23
| | | | | | | |
* | | | | | | | Merge branch 'fix/status-of-pipeline-without-builds' into 'master' Rémy Coutable2016-06-163-3/+39
|\ \ \ \ \ \ \ \ | |/ / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Improve pipeline status in case that pipeline has no jobs ## What does this MR do? This MR resolves problem with pipeline status when there are no build in pipeline. This can happen when builds were skipped - for example - by using `only`/`except` keyword in `.gitlab-ci.yml`. ## What are the relevant issue numbers? Closes #17977 See merge request !4403
| * | | | | | | Merge branch 'master' into fix/status-of-pipeline-without-buildsfix/status-of-pipeline-without-buildsGrzegorz Bizon2016-06-1558-207/+2802
| |\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * master: (198 commits) Set inverse_of for Project/Services relation Fix admin hooks spec Prevent default disabled buttons and links. Add index on `requested_at` to the `members` table Rearrange order of tabs Fix admin active tab tests Show created_at in table column Nest li elements directly under ul Move builds tab to admin overview Add monitoring link with subtabs Add sub links to overview Add counter for abuse reports Remove admin layout-nav counters Move admin nav to horizontal layout nav Eager load project relations in IssueParser Use validate and required for environment and project Award Emoji can't be awarded on system notes backend Get rid of Gitlab::ShellEnv Update CHANGELOG. Fix project star tooltip on the fly. ... Conflicts: app/services/ci/create_builds_service.rb
| * | | | | | | | Remove reduntant method for building pipeline buildsGrzegorz Bizon2016-06-152-0/+4
| | | | | | | | |
| * | | | | | | | Return false in create_builds if not builds createdGrzegorz Bizon2016-06-151-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This fixes compatibility with trigger request create service.
| * | | | | | | | Improve code clarity in pipeline create serviceGrzegorz Bizon2016-06-141-2/+2
| | | | | | | | |
| * | | | | | | | Remove ci commit specs that remain after bad mergeGrzegorz Bizon2016-06-141-403/+0
| | | | | | | | |
| * | | | | | | | Merge branch 'master' into fix/status-of-pipeline-without-buildsGrzegorz Bizon2016-06-14150-1087/+6017
| |\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * master: (538 commits) Fix broken URI joining for `teamcity_url` with suffixes Factorize duplicated code into a method in BambooService and update specs Fix broken URI joining for `bamboo_url` with suffixes Honor credentials on calling Bamboo CI trigger Update CHANGELOG Use Issue.visible_to_user in Notes.search to avoid query duplication Project members with guest role can't access confidential issues Allow users to create confidential issues in private projects Update CHANGELOG Remove deprecated issues_tracker and issues_tracker_id from project Schema doesn’t reflect the changes of the last 3 migrations Apply reviewer notes: update CHANGELOG, adjust code formatting Move issue rendering tests into separate contexts Move change description to proper release and fix typo Add more information into RSS fead for issues Revert CHANGELOG Also rename "find" in the specs Change to new Notes styleguide Add guide on changing a document's location Change logs.md location in README ... Conflicts: app/services/ci/create_builds_service.rb app/services/ci/create_pipeline_service.rb app/services/create_commit_builds_service.rb spec/models/ci/commit_spec.rb spec/services/ci/create_builds_service_spec.rb spec/services/create_commit_builds_service_spec.rb
| * | | | | | | | | Refactor code reponsible for creating buildsGrzegorz Bizon2016-06-033-15/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This removes duplications and extracts method that builds build-jobs without persisting those objects, to a separate method.
| * | | | | | | | | Fix specs for pipeline create for merge requestsGrzegorz Bizon2016-06-031-1/+4
| | | | | | | | | |
| * | | | | | | | | Update specs describeKamil Trzcinski2016-06-031-1/+1
| | | | | | | | | |
| * | | | | | | | | Update CreateCommitBuildsService to pass testsKamil Trzcinski2016-06-032-7/+7
| | | | | | | | | |
| * | | | | | | | | Save Ci::Commit object to persist all created buildsKamil Trzcinski2016-06-032-2/+11
| | | | | | | | | |
| * | | | | | | | | Do not create pipeline objects when no buildsGrzegorz Bizon2016-06-031-1/+5
| | | | | | | | | |
| * | | | | | | | | Update ci commit pipeline specs according to changesGrzegorz Bizon2016-06-032-9/+9
| | | | | | | | | |
| * | | | | | | | | Add minor improvements in create builds serviceGrzegorz Bizon2016-06-031-1/+1
| | | | | | | | | |
| * | | | | | | | | Add specs covering case when there are no buildsGrzegorz Bizon2016-06-031-0/+18
| | | | | | | | | |
| * | | | | | | | | Refactor ci commit pipeline to prevent implicit savesGrzegorz Bizon2016-06-031-1/+1
| | | | | | | | | |
* | | | | | | | | | Merge branch '18591-banzai-filter-upload-link-filter' into 'master' Yorick Peterse2016-06-161-2/+18
|\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Banzai::Filter::UploadLinkFilter use XPath See merge request !4703
| * | | | | | | | | | Banzai::Filter::UploadLinkFilter use XPathPaco Guzman2016-06-161-2/+18
| | | | | | | | | | |
* | | | | | | | | | | Merge branch 'backport-view-condition-improvement-from-ee-460' into 'master' Douwe Maan2016-06-161-16/+0
|\ \ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix permission checks in member row (backport from gitlab-org/gitlab-ee!460) ## What does this MR do? It improves the check we use to display or not the members' access and controls in the members list. ## Are there points in the code the reviewer needs to double check? No, I replaced an helper with just a permission check so I think it's a better solution. ## Why was this MR needed? There were a spec failure in gitlab-org/gitlab-ee!460 because of the refactor done in the "request access" MR. ## What are the relevant issue numbers? None. ## Does this MR meet the acceptance criteria? - No CHANGELOG needed - [x] Tests - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4670
| * | | | | | | | | | | Fix permission checks in member rowbackport-view-condition-improvement-from-ee-460Rémy Coutable2016-06-151-16/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Rémy Coutable <remy@rymai.me>
* | | | | | | | | | | | Merge branch 'group-owners-association' into 'master' Rémy Coutable2016-06-161-0/+12
|\ \ \ \ \ \ \ \ \ \ \ \ | |_|/ / / / / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Turn Group#owners into a has_many association ## What does this MR do? This turns the regular method `Group#owners` into a `has_many` association. ## Are there points in the code the reviewer needs to double check? As far as I can tell there's no way to do this without using an intermediate association, but perhaps I'm missing something. The reason an intermediate association is needed is because the supplied Proc is applied to the _final_ association (the one returning users), this means that when using a single `has_many` you can't filter out any intermediate rows (e.g. group members). ## Why was this MR needed? This code being a regular method would prevent eager loading of the owners of a Group, turning it into a `has_many` association resolves this problem. This was discovered in !4410. ## What are the relevant issue numbers? None. ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [x] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - [x] ~~API support added~~ - [ ] Tests - [x] Added for this feature/bug - [ ] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [ ] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4676
| * | | | | | | | | | | Turn Group#owners into a has_many associationYorick Peterse2016-06-161-0/+12
| |/ / / / / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This allows the owners to be eager loaded where needed.
* | | | | | | | | | | Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into ↵Fatih Acet2016-06-1678-294/+3147
|\ \ \ \ \ \ \ \ \ \ \ | | |_|_|_|/ / / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | category-search-dropdown # Conflicts: # app/assets/javascripts/lib/common_utils.js.coffee
| * | | | | | | | | | Merge branch 'issuable-sidebar-bold' into 'master' Jacob Schatz2016-06-151-2/+2
| |\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixed issue with bold in issuable sidebar ## What does this MR do? ~~The sidebar contained both normal font weight & bold font weight, so this standardises them to bold.~~ After looking at the designs, i've updated the font weights & colors throughout the sidebar to correctly match them ## Screenshots (if relevant) ![Screen_Shot_2016-06-02_at_11.03.40](/uploads/0a3eb6a67ce5722c77f6adf2fe883017/Screen_Shot_2016-06-02_at_11.03.40.png) See merge request !4398
| | * | | | | | | | | | Fixed testsPhil Hughes2016-06-101-2/+2
| | | | | | | | | | | |
| * | | | | | | | | | | Merge branch 'fair-usage-of-shared-runners' into 'master' Stan Hu2016-06-151-0/+62
| |\ \ \ \ \ \ \ \ \ \ \ | | |_|_|_|_|/ / / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fair usage of Shared Runners ## What does this MR do? Introduces a fair usage scheduler for shared runners. It tries to assign builds to shared runner from projects that have the lowest number of builds currently running on shared runners. **Example 1**: ``` We have following builds in queue: build 1 for project 1 build 2 for project 1 build 3 for project 1 build 4 for project 2 build 5 for project 2 build 6 for project 3 With the new algorithm we will assign builds in following order: - We choose build 1, because project 1 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We choose build 4, because project 2 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We choose build 6, because project 3 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We choose build 2, because project 1 as other it runs 1 build, - We choose build 5, because project 2 runs 1 build, where project 1 runs 2 builds now, - We choose build 3, because project 1 and runs 2 builds. ``` **Example 2**: ``` We have following builds in queue: build 1 for project 1 build 2 for project 1 build 3 for project 1 build 4 for project 2 build 5 for project 2 build 6 for project 3 With the new algorithm we will assign builds in following order: - We choose build 1, because project 1 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We finish build 1, - We choose build 2, because project 1 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We choose build 4, because project 2 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We finish build 4, - We choose build 5, because project 2 doesn't run currently any builds and has the lowest build number from projects that doesn't run builds, - We choose build 6, because project 3 doesn't run currently any builds, - We choose build 3, because project 1, 2 and 3 runs exactly one build now, ``` ## Why was this MR needed? Currently, we are scheduling builds using FIFO. This is catastrophic if there are projects that create a 100-300 jobs, this basically eats most of available shared runners. ## Performance All this logic is implemented with the help of SQL queries, because this is the fastest way to process 1k-2k pending builds in queue. It's not the fastest SQL query, because it sorts based on number of running_builds, and this forces to calculate a number of running builds for all dependent projects. However, since we have one/two shared runners that asks every few seconds for builds this should have minimal impact on DB performance. ``` explain analyze SELECT "ci_builds".* FROM "ci_builds" JOIN (SELECT "ci_builds"."gl_project_id", count(case when status = 'running' AND runner_id = (SELECT "ci_runners"."id" FROM "ci_runners" WHERE "ci_runners"."is_shared" = 't') then 1 end) as running_builds FROM "ci_builds" INNER JOIN "projects" ON "projects"."id" = "ci_builds"."gl_project_id" AND "projects"."pending_delete" = 'f' WHERE "ci_builds"."type" IN ('Ci::Build') AND "ci_builds"."status" IN ('running', 'pending') AND "projects"."builds_enabled" = 't' AND "projects"."shared_runners_enabled" = 't' GROUP BY "ci_builds"."gl_project_id") AS projects ON ci_builds.gl_project_id=projects.gl_project_id WHERE "ci_builds"."type" IN ('Ci::Build') AND "ci_builds"."status" = 'pending' AND "ci_builds"."runner_id" IS NULL ORDER BY projects.running_builds ASC, ci_builds.id ASC; QUERY PLAN -------------------------------------------------------------------------------------------------------------------------------------------------------------------- ----------- Sort (cost=64777.28..64777.29 rows=1 width=1010) (actual time=301.794..302.535 rows=1537 loops=1) Sort Key: (count(CASE WHEN (((public.ci_builds.status)::text = 'running'::text) AND (public.ci_builds.runner_id = $0)) THEN 1 ELSE NULL::integer END)), public.ci _builds.id Sort Method: quicksort Memory: 1423kB -> Nested Loop (cost=63279.78..64777.27 rows=1 width=1010) (actual time=66.384..298.724 rows=1537 loops=1) -> HashAggregate (cost=63177.15..63177.30 rows=15 width=15) (actual time=65.641..65.851 rows=187 loops=1) InitPlan 1 (returns $0) -> Seq Scan on ci_runners (cost=0.00..26963.66 rows=1 width=4) (actual time=1.145..34.381 rows=1 loops=1) Filter: is_shared Rows Removed by Filter: 6965 -> Nested Loop (cost=0.00..36186.34 rows=2715 width=15) (actual time=0.065..29.717 rows=1710 loops=1) -> Index Scan using index_ci_builds_on_status on ci_builds (cost=0.00..8913.95 rows=3577 width=15) (actual time=0.051..12.012 rows=2583 loops =1) Index Cond: ((status)::text = ANY ('{running,pending}'::text[])) Filter: ((type)::text = 'Ci::Build'::text) Rows Removed by Filter: 1219 -> Index Scan using projects_pkey on projects (cost=0.00..7.61 rows=1 width=4) (actual time=0.003..0.004 rows=1 loops=2583) Index Cond: (id = public.ci_builds.gl_project_id) Filter: ((NOT pending_delete) AND builds_enabled AND shared_runners_enabled) Rows Removed by Filter: 0 -> Bitmap Heap Scan on ci_builds (cost=102.63..106.64 rows=1 width=1002) (actual time=1.216..1.231 rows=8 loops=187) Recheck Cond: ((gl_project_id = public.ci_builds.gl_project_id) AND ((status)::text = 'pending'::text)) Filter: ((runner_id IS NULL) AND ((type)::text = 'Ci::Build'::text)) -> BitmapAnd (cost=102.63..102.63 rows=1 width=0) (actual time=1.201..1.201 rows=0 loops=187) -> Bitmap Index Scan on index_ci_builds_on_gl_project_id (cost=0.00..10.52 rows=241 width=0) (actual time=0.406..0.406 rows=1944 loops=187) Index Cond: (gl_project_id = public.ci_builds.gl_project_id) -> Bitmap Index Scan on index_ci_builds_on_status (cost=0.00..91.78 rows=3089 width=0) (actual time=0.652..0.652 rows=3362 loops=187) Index Cond: ((status)::text = 'pending'::text) Total runtime: 303.832 ms ``` ## Specific runners It doesn't affect the specific runners which still serve builds FIFO. @stanhu @markpundsack @yorickpeterse What do you think? See merge request !4634
| | * | | | | | | | | | Make sure that we test RegisterBuildService behavior for deleted projectsfair-usage-of-shared-runnersKamil Trzcinski2016-06-151-0/+22
| | | | | | | | | | | |
| | * | | | | | | | | | Fair usage of Shared RunnersKamil Trzcinski2016-06-131-0/+40
| | | | | | | | | | | |