diff options
Diffstat (limited to 'doc/development')
72 files changed, 1758 insertions, 1093 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index 6281bb809ff..3912a828dec 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -5,7 +5,7 @@ description: 'Learn how to contribute to GitLab.' # Contributor and Development Docs -## Get started! +## Get started - Set up GitLab's development environment with [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md) - [GitLab contributing guide](contributing/index.md) @@ -65,6 +65,7 @@ description: 'Learn how to contribute to GitLab.' - [Repository mirroring](repository_mirroring.md) - [Git LFS](lfs.md) - [Developing against interacting components or features](interacting_components.md) +- [File uploads](uploads.md) ## Performance guides @@ -116,6 +117,7 @@ description: 'Learn how to contribute to GitLab.' ## Case studies - [Database case study: Filtering by label](filtering_by_label.md) +- [Database case study: Namespaces storage statistics](namespaces_storage_statistics.md) ## Integration guides diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index 0866d3baeeb..61576236c96 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -51,7 +51,7 @@ allowed. – <https://github.com/ruby-grape/grape#declared> -### Exclude params from parent namespaces! +### Exclude params from parent namespaces > By default `declared(params)`includes parameters that were defined in all parent namespaces. @@ -64,7 +64,7 @@ In most cases you will want to exclude params from the parent namespaces: declared(params, include_parent_namespaces: false) ``` -### When to use `declared(params)`? +### When to use `declared(params)` You should always use `declared(params)` when you pass the params hash as arguments to a method call. diff --git a/doc/development/architecture.md b/doc/development/architecture.md index 87405bc2fec..2adca2dae28 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -12,21 +12,21 @@ New versions of GitLab are released in stable branches and the master branch is For information, see the [GitLab Release Process](https://gitlab.com/gitlab-org/release/docs/tree/master#gitlab-release-process). -Both EE and CE require some add-on components called gitlab-shell and Gitaly. These components are available from the [gitlab-shell](https://gitlab.com/gitlab-org/gitlab-shell/tree/master) and [gitaly](https://gitlab.com/gitlab-org/gitaly/tree/master) repositories respectively. New versions are usually tags but staying on the master branch will give you the latest stable version. New releases are generally around the same time as GitLab CE releases with exception for informal security updates deemed critical. +Both EE and CE require some add-on components called GitLab Shell and Gitaly. These components are available from the [GitLab Shell](https://gitlab.com/gitlab-org/gitlab-shell/tree/master) and [Gitaly](https://gitlab.com/gitlab-org/gitaly/tree/master) repositories respectively. New versions are usually tags but staying on the master branch will give you the latest stable version. New releases are generally around the same time as GitLab CE releases with exception for informal security updates deemed critical. ## Components A typical install of GitLab will be on GNU/Linux. It uses Nginx or Apache as a web front end to proxypass the Unicorn web server. By default, communication between Unicorn and the front end is via a Unix domain socket but forwarding requests via TCP is also supported. The web front end accesses `/home/git/gitlab/public` bypassing the Unicorn server to serve static pages, uploads (e.g. avatar images or attachments), and precompiled assets. GitLab serves web pages and a [GitLab API](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/api) using the Unicorn web server. It uses Sidekiq as a job queue which, in turn, uses redis as a non-persistent database backend for job information, meta data, and incoming jobs. -We also support deploying GitLab on Kubernetes using our [gitlab Helm chart](https://docs.gitlab.com/charts/). +We also support deploying GitLab on Kubernetes using our [GitLab Helm chart](https://docs.gitlab.com/charts/). -The GitLab web app uses MySQL or PostgreSQL for persistent database information (e.g. users, permissions, issues, other meta data). GitLab stores the bare git repositories it serves in `/home/git/repositories` by default. It also keeps default branch and hook information with the bare repository. +The GitLab web app uses PostgreSQL for persistent database information (e.g. users, permissions, issues, other meta data). GitLab stores the bare Git repositories it serves in `/home/git/repositories` by default. It also keeps default branch and hook information with the bare repository. -When serving repositories over HTTP/HTTPS GitLab utilizes the GitLab API to resolve authorization and access as well as serving git objects. +When serving repositories over HTTP/HTTPS GitLab utilizes the GitLab API to resolve authorization and access as well as serving Git objects. -The add-on component gitlab-shell serves repositories over SSH. It manages the SSH keys within `/home/git/.ssh/authorized_keys` which should not be manually edited. gitlab-shell accesses the bare repositories through Gitaly to serve git objects and communicates with redis to submit jobs to Sidekiq for GitLab to process. gitlab-shell queries the GitLab API to determine authorization and access. +The add-on component GitLab Shell serves repositories over SSH. It manages the SSH keys within `/home/git/.ssh/authorized_keys` which should not be manually edited. GitLab Shell accesses the bare repositories through Gitaly to serve Git objects and communicates with redis to submit jobs to Sidekiq for GitLab to process. GitLab Shell queries the GitLab API to determine authorization and access. -Gitaly executes git operations from gitlab-shell and the GitLab web app, and provides an API to the GitLab web app to get attributes from git (e.g. title, branches, tags, other meta data), and to get blobs (e.g. diffs, commits, files). +Gitaly executes Git operations from GitLab Shell and the GitLab web app, and provides an API to the GitLab web app to get attributes from Git (e.g. title, branches, tags, other meta data), and to get blobs (e.g. diffs, commits, files). You may also be interested in the [production architecture of GitLab.com](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/). @@ -130,7 +130,7 @@ Component statuses are linked to configuration documentation for each component. | [NGINX](#nginx) | Routes requests to appropriate components, terminates SSL | [✅][nginx-omnibus] | [✅][nginx-charts] | [⚙][nginx-charts] | [✅](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/#service-architecture) | [⤓][nginx-source] | ❌ | CE & EE | | [Unicorn (GitLab Rails)](#unicorn) | Handles requests for the web interface and API | [✅][unicorn-omnibus] | [✅][unicorn-charts] | [✅][unicorn-charts] | [✅](../user/gitlab_com/index.md#unicorn) | [⚙][unicorn-source] | [✅][gitlab-yml] | CE & EE | | [Sidekiq](#sidekiq) | Background jobs processor | [✅][sidekiq-omnibus] | [✅][sidekiq-charts] | [✅](https://docs.gitlab.com/charts/charts/gitlab/sidekiq/index.html) | [✅](../user/gitlab_com/index.md#sidekiq) | [✅][gitlab-yml] | [✅][gitlab-yml] | CE & EE | -| [Gitaly](#gitaly) | Git RPC service for handling all git calls made by GitLab | [✅][gitaly-omnibus] | [✅][gitaly-charts] | [✅][gitaly-charts] | [✅](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/#service-architecture) | [⚙][gitaly-source] | ✅ | CE & EE | +| [Gitaly](#gitaly) | Git RPC service for handling all Git calls made by GitLab | [✅][gitaly-omnibus] | [✅][gitaly-charts] | [✅][gitaly-charts] | [✅](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/#service-architecture) | [⚙][gitaly-source] | ✅ | CE & EE | | [GitLab Workhorse](#gitlab-workhorse) | Smart reverse proxy, handles large HTTP requests | [✅][workhorse-omnibus] | [✅][workhorse-charts] | [✅][workhorse-charts] | [✅](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/#service-architecture) | [⚙][workhorse-source] | ✅ | CE & EE | | [GitLab Shell](#gitlab-shell) | Handles `git` over SSH sessions | [✅][shell-omnibus] | [✅][shell-charts] | [✅][shell-charts] | [✅](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/#service-architecture) | [⚙][shell-source] | [✅][gitlab-yml] | CE & EE | | [GitLab Pages](#gitlab-pages) | Hosts static websites | [⚙][pages-omnibus] | [❌][pages-charts] | [❌][pages-charts] | [✅](../user/gitlab_com/index.md#gitlab-pages) | [⚙][pages-source] | [⚙][pages-gdk] | CE & EE | @@ -185,7 +185,7 @@ GitLab can be considered to have two layers from a process perspective: - Layer: Monitoring - Process: `alertmanager` -[Alert manager](https://prometheus.io/docs/alerting/alertmanager/) is a tool provided by Prometheus that _"handles alerts sent by client applications such as the Prometheus server. It takes care of deduplicating, grouping, and routing them to the correct receiver integration such as email, PagerDuty, or OpsGenie. It also takes care of silencing and inhibition of alerts."_ You can read more in [issue gitlab-ce#45740](https://gitlab.com/gitlab-org/gitlab-ce/issues/45740) about what we will be alerting on. +[Alert manager](https://prometheus.io/docs/alerting/alertmanager/) is a tool provided by Prometheus that _"handles alerts sent by client applications such as the Prometheus server. It takes care of deduplicating, grouping, and routing them to the correct receiver integration such as email, PagerDuty, or OpsGenie. It also takes care of silencing and inhibition of alerts."_ You can read more in [issue #45740](https://gitlab.com/gitlab-org/gitlab-ce/issues/45740) about what we will be alerting on. #### Certificate management @@ -223,12 +223,12 @@ Elasticsearch is a distributed RESTful search engine built for the cloud. Gitaly is a service designed by GitLab to remove our need for NFS for Git storage in distributed deployments of GitLab (think GitLab.com or High Availability Deployments). As of 11.3.0, this service handles all Git level access in GitLab. You can read more about the project [in the project's readme](https://gitlab.com/gitlab-org/gitaly). -#### Gitlab Geo +#### GitLab Geo - Configuration: [Omnibus][geo-omnibus], [Charts][geo-charts], [GDK][geo-gdk] - Layer: Core Service (Processor) -#### Gitlab Monitor +#### GitLab Monitor - [Project page](https://gitlab.com/gitlab-org/gitlab-monitor) - Configuration: [Omnibus][gitlab-monitor-omnibus], [Charts][gitlab-monitor-charts] @@ -237,7 +237,7 @@ Gitaly is a service designed by GitLab to remove our need for NFS for Git storag GitLab Monitor is a process designed in house that allows us to export metrics about GitLab application internals to Prometheus. You can read more [in the project's readme](https://gitlab.com/gitlab-org/gitlab-monitor). -#### Gitlab Pages +#### GitLab Pages - Configuration: [Omnibus][pages-omnibus], [Charts][pages-charts], [Source][pages-source], [GDK][pages-gdk] - Layer: Core Service (Processor) @@ -246,7 +246,7 @@ GitLab Pages is a feature that allows you to publish static websites directly fr You can use it either for personal or business websites, such as portfolios, documentation, manifestos, and business presentations. You can also attribute any license to your content. -#### Gitlab Runner +#### GitLab Runner - [Project page](https://gitlab.com/gitlab-org/gitlab-runner/blob/master/README.md) - Configuration: [Omnibus][runner-omnibus], [Charts][runner-charts], [Source][runner-source], [GDK][runner-gdk] @@ -256,7 +256,7 @@ GitLab Runner runs tests and sends the results to GitLab. GitLab CI is the open-source continuous integration service included with GitLab that coordinates the testing. The old name of this project was GitLab CI Multi Runner but please use "GitLab Runner" (without CI) from now on. -#### Gitlab Shell +#### GitLab Shell - [Project page](https://gitlab.com/gitlab-org/gitlab-shell/blob/master/README.md) - Configuration: [Omnibus][shell-omnibus], [Charts][shell-charts], [Source][shell-source], [GDK][gitlab-yml] @@ -264,7 +264,7 @@ GitLab CI is the open-source continuous integration service included with GitLab [GitLab Shell](https://gitlab.com/gitlab-org/gitlab-shell) is a program designed at GitLab to handle ssh-based `git` sessions, and modifies the list of authorized keys. GitLab Shell is not a Unix shell nor a replacement for Bash or Zsh. -#### Gitlab Workhorse +#### GitLab Workhorse - [Project page](https://gitlab.com/gitlab-org/gitlab-workhorse/blob/master/README.md) - Configuration: [Omnibus][workhorse-omnibus], [Charts][workhorse-charts], [Source][workhorse-source] @@ -475,7 +475,7 @@ It's important to understand the distinction as some processes are used in both When making a request to an HTTP Endpoint (think `/users/sign_in`) the request will take the following path through the GitLab Service: - nginx - Acts as our first line reverse proxy. -- gitlab-workhorse - This determines if it needs to go to the Rails application or somewhere else to reduce load on Unicorn. +- GitLab Workhorse - This determines if it needs to go to the Rails application or somewhere else to reduce load on Unicorn. - unicorn - Since this is a web request, and it needs to access the application it will go to Unicorn. - Postgres/Gitaly/Redis - Depending on the type of request, it may hit these services to store or retrieve data. @@ -493,13 +493,13 @@ TODO ## System Layout -When referring to `~git` in the pictures it means the home directory of the git user which is typically `/home/git`. +When referring to `~git` in the pictures it means the home directory of the Git user which is typically `/home/git`. GitLab is primarily installed within the `/home/git` user home directory as `git` user. Within the home directory is where the gitlabhq server software resides as well as the repositories (though the repository location is configurable). The bare repositories are located in `/home/git/repositories`. GitLab is a ruby on rails application so the particulars of the inner workings can be learned by studying how a ruby on rails application works. -To serve repositories over SSH there's an add-on application called gitlab-shell which is installed in `/home/git/gitlab-shell`. +To serve repositories over SSH there's an add-on application called GitLab Shell which is installed in `/home/git/gitlab-shell`. ### Installation Folder Summary @@ -511,11 +511,19 @@ To summarize here's the [directory structure of the `git` user home directory](. ps aux | grep '^git' ``` -GitLab has several components to operate. As a system user (i.e. any user that is not the `git` user) it requires a persistent database (MySQL/PostreSQL) and redis database. It also uses Apache httpd or Nginx to proxypass Unicorn. As the `git` user it starts Sidekiq and Unicorn (a simple ruby HTTP server running on port `8080` by default). Under the GitLab user there are normally 4 processes: `unicorn_rails master` (1 process), `unicorn_rails worker` (2 processes), `sidekiq` (1 process). +GitLab has several components to operate. It requires a persistent database +(PostgreSQL) and redis database, and uses Apache httpd or Nginx to proxypass +Unicorn. All these components should run as different system users to GitLab +(e.g., `postgres`, `redis` and `www-data`, instead of `git`). + +As the `git` user it starts Sidekiq and Unicorn (a simple ruby HTTP server +running on port `8080` by default). Under the GitLab user there are normally 4 +processes: `unicorn_rails master` (1 process), `unicorn_rails worker` +(2 processes), `sidekiq` (1 process). ### Repository access -Repositories get accessed via HTTP or SSH. HTTP cloning/push/pull utilizes the GitLab API and SSH cloning is handled by gitlab-shell (previously explained). +Repositories get accessed via HTTP or SSH. HTTP cloning/push/pull utilizes the GitLab API and SSH cloning is handled by GitLab Shell (previously explained). ## Troubleshooting @@ -523,28 +531,28 @@ See the README for more information. ### Init scripts of the services -The GitLab init script starts and stops Unicorn and Sidekiq. +The GitLab init script starts and stops Unicorn and Sidekiq: ``` /etc/init.d/gitlab Usage: service gitlab {start|stop|restart|reload|status} ``` -Redis (key-value store/non-persistent database) +Redis (key-value store/non-persistent database): ``` /etc/init.d/redis Usage: /etc/init.d/redis {start|stop|status|restart|condrestart|try-restart} ``` -SSH daemon +SSH daemon: ``` /etc/init.d/sshd Usage: /etc/init.d/sshd {start|stop|restart|reload|force-reload|condrestart|try-restart|status} ``` -Web server (one of the following) +Web server (one of the following): ``` /etc/init.d/httpd @@ -554,54 +562,46 @@ $ /etc/init.d/nginx Usage: nginx {start|stop|restart|reload|force-reload|status|configtest} ``` -Persistent database (one of the following) +Persistent database: ``` -/etc/init.d/mysqld -Usage: /etc/init.d/mysqld {start|stop|status|restart|condrestart|try-restart|reload|force-reload} - $ /etc/init.d/postgresql Usage: /etc/init.d/postgresql {start|stop|restart|reload|force-reload|status} [version ..] ``` ### Log locations of the services -gitlabhq (includes Unicorn and Sidekiq logs) +gitlabhq (includes Unicorn and Sidekiq logs): - `/home/git/gitlab/log/` contains `application.log`, `production.log`, `sidekiq.log`, `unicorn.stdout.log`, `git_json.log` and `unicorn.stderr.log` normally. -gitlab-shell +GitLab Shell: - `/home/git/gitlab-shell/gitlab-shell.log` -ssh +SSH: - `/var/log/auth.log` auth log (on Ubuntu). - `/var/log/secure` auth log (on RHEL). -nginx +nginx: - `/var/log/nginx/` contains error and access logs. -Apache httpd +Apache httpd: - [Explanation of Apache logs](https://httpd.apache.org/docs/2.2/logs.html). - `/var/log/apache2/` contains error and output logs (on Ubuntu). - `/var/log/httpd/` contains error and output logs (on RHEL). -redis +Redis: - `/var/log/redis/redis.log` there are also log-rotated logs there. -PostgreSQL +PostgreSQL: - `/var/log/postgresql/*` -MySQL - -- `/var/log/mysql/*` -- `/var/log/mysql.*` - ### GitLab specific config files GitLab has configuration files located in `/home/git/gitlab/config/*`. Commonly referenced config files include: @@ -610,7 +610,7 @@ GitLab has configuration files located in `/home/git/gitlab/config/*`. Commonly - `unicorn.rb` - Unicorn web server settings. - `database.yml` - Database connection settings. -gitlab-shell has a configuration file at `/home/git/gitlab-shell/config.yml`. +GitLab Shell has a configuration file at `/home/git/gitlab-shell/config.yml`. ### Maintenance Tasks diff --git a/doc/development/automatic_ce_ee_merge.md b/doc/development/automatic_ce_ee_merge.md index 98b8a48abf4..c2700461467 100644 --- a/doc/development/automatic_ce_ee_merge.md +++ b/doc/development/automatic_ce_ee_merge.md @@ -173,13 +173,13 @@ Now, every time you create an MR for CE and EE: ## How we run the Automatic CE->EE merge at GitLab -At GitLab, we use the [Merge Train](https://gitlab.com/gitlab-org/merge-train) -project to keep our [gitlab-ee](https://gitlab.com/gitlab-org/gitlab-ee) -repository updated with commits from -[gitlab-ce](https://gitlab.com/gitlab-org/gitlab-ce). +At GitLab, we use the [Merge Train](https://gitlab.com/gitlab-org/merge-train) +project to keep our [GitLab EE](https://gitlab.com/gitlab-org/gitlab-ee) +repository updated with commits from +[GitLab CE](https://gitlab.com/gitlab-org/gitlab-ce). We have a mirror of the [Merge Train](https://gitlab.com/gitlab-org/merge-train) -project [configured](https://ops.gitlab.net/gitlab-org/merge-train) to run an +project [configured](https://ops.gitlab.net/gitlab-org/merge-train) to run an automatic CE->EE merge job every twenty minutes as a scheduled CI job. The [configured](https://ops.gitlab.net/gitlab-org/merge-train) Merge Train project is only accessible to authorized GitLab staff. @@ -200,7 +200,7 @@ code. ### Why merge automatically? As we work towards continuous deployments and a single repository for both CE -and EE, we need to first make sure that all CE changes make their way into CE as +and EE, we need to first make sure that all CE changes make their way into EE as fast as possible. Past experiences and data have shown that periodic CE to EE merge requests do not scale, and often take a very long time to complete. For example, [in this diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 642dac614c7..3fd95537eaa 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -294,7 +294,7 @@ to migrate you database down and up, which can result in other background migrations being called. That means that using `spy` test doubles with `have_received` is encouraged, instead of using regular test doubles, because your expectations defined in a `it` block can conflict with what is being -called in RSpec hooks. See [gitlab-org/gitlab-ce#35351][issue-rspec-hooks] +called in RSpec hooks. See [issue #35351][issue-rspec-hooks] for more details. ## Best practices diff --git a/doc/development/build_test_package.md b/doc/development/build_test_package.md index c5f6adfeaeb..21891f70d73 100644 --- a/doc/development/build_test_package.md +++ b/doc/development/build_test_package.md @@ -3,7 +3,7 @@ While developing a new feature or modifying an existing one, it is helpful if an installable package (or a docker image) containing those changes is available for testing. For this very purpose, a manual job is provided in the GitLab CI/CD -pipeline that can be used to trigger a pipeline in the omnibus-gitlab repository +pipeline that can be used to trigger a pipeline in the Omnibus GitLab repository that will create: - A deb package for Ubuntu 16.04, available as a build artifact, and @@ -12,7 +12,7 @@ that will create: (images titled `gitlab-ce` and `gitlab-ee` respectively and image tag is the commit which triggered the pipeline). -When you push a commit to either the gitlab-ce or gitlab-ee project, the +When you push a commit to either the GitLab CE or GitLab EE project, the pipeline for that commit will have a `build-package` manual action you can trigger. @@ -30,9 +30,9 @@ branch `0-1-stable`, modify the content of `GITALY_SERVER_VERSION` to `0-1-stable` and push the commit. This will create a manual job that can be used to trigger the build. -## Specifying the branch in omnibus-gitlab repository +## Specifying the branch in Omnibus GitLab repository -In scenarios where a configuration change is to be introduced and omnibus-gitlab +In scenarios where a configuration change is to be introduced and Omnibus GitLab repository already has the necessary changes in a specific branch, you can build a package against that branch through an environment variable named `OMNIBUS_BRANCH`. To do this, specify that environment variable with the name of diff --git a/doc/development/changelog.md b/doc/development/changelog.md index bd07a01e782..814624c7586 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -35,6 +35,7 @@ the `author` field. GitLab team members **should not**. - Any user-facing change **should** have a changelog entry. Example: "GitLab now uses system fonts for all text." +- Any docs-only changes **should not** have a changelog entry. - Any change behind a feature flag **should not** have a changelog entry. The entry should be added [in the merge request removing the feature flags](feature_flags/development.md). - A fix for a regression introduced and then fixed in the same release (i.e., fixing a bug introduced during a monthly release candidate) **should not** diff --git a/doc/development/contributing/index.md b/doc/development/contributing/index.md index 853882e8642..887f17b05b8 100644 --- a/doc/development/contributing/index.md +++ b/doc/development/contributing/index.md @@ -65,7 +65,7 @@ Sign up for the mailing list, answer GitLab questions on StackOverflow or respond in the IRC channel. You can also sign up on [CodeTriage][codetriage] to help with the remaining issues on the GitHub issue tracker. -## I want to contribute! +## I want to contribute If you want to contribute to GitLab, [issues with the `Accepting merge requests` label](issue_workflow.md#label-for-community-contributors) @@ -93,26 +93,20 @@ When submitting code to GitLab, you may feel that your contribution requires the When your code contains more than 500 changes, any major breaking changes, or an external library, `@mention` a maintainer in the merge request. If you are not sure who to mention, the reviewer will add one early in the merge request process. -## Issues +## Issues workflow -This [documentation](issue_workflow.md) outlines the current issue process. +This [documentation](issue_workflow.md) outlines the current issue workflow: -- [Type labels](issue_workflow.md#type-labels) -- [Subject labels](issue_workflow.md#subject-labels) -- [Team labels](issue_workflow.md#team-labels) -- [Release Scoping labels](issue_workflow.md#release-scoping-labels) -- [Priority labels](issue_workflow.md#priority-labels) -- [Severity labels](issue_workflow.md#severity-labels) -- [Label for community contributors](issue_workflow.md#label-for-community-contributors) +- [Issue tracker guidelines](issue_workflow.md#issue-tracker-guidelines) - [Issue triaging](issue_workflow.md#issue-triaging) +- [Labels](issue_workflow.md#labels) - [Feature proposals](issue_workflow.md#feature-proposals) -- [Issue tracker guidelines](issue_workflow.md#issue-tracker-guidelines) - [Issue weight](issue_workflow.md#issue-weight) - [Regression issues](issue_workflow.md#regression-issues) - [Technical and UX debt](issue_workflow.md#technical-and-ux-debt) -- [Stewardship](issue_workflow.md#stewardship) +- [Technical debt in follow-up issues](issue_workflow.md#technical-debt-in-follow-up-issues) -## Merge requests +## Merge requests workflow This [documentation](merge_request_workflow.md) outlines the current merge request process. diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index a38794c49af..8b5d380ad9e 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -1,17 +1,51 @@ -# Workflow labels +# Issues workflow -To allow for asynchronous issue handling, we use [milestones][milestones-page] +## Issue tracker guidelines + +**[Search the issue tracker](https://gitlab.com/gitlab-org/gitlab-ce/issues)** for similar entries before +submitting your own, there's a good chance somebody else had the same issue or +feature proposal. Show your support with an award emoji and/or join the +discussion. + +Please submit bugs using the ['Bug' issue template](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.gitlab/issue_templates/Bug.md) provided on the issue tracker. +The text in the parenthesis is there to help you with what to include. Omit it +when submitting the actual issue. You can copy-paste it and then edit as you +see fit. + +## Issue triaging + +Our issue triage policies are [described in our handbook](https://about.gitlab.com/handbook/engineering/issue-triage/). +You are very welcome to help the GitLab team triage issues. +We also organize [issue bash events](https://gitlab.com/gitlab-org/gitlab-ce/issues/17815) +once every quarter. + +The most important thing is making sure valid issues receive feedback from the +development team. Therefore the priority is mentioning developers that can help +on those issues. Please select someone with relevant experience from the +[GitLab team](https://about.gitlab.com/team/). +If there is nobody mentioned with that expertise look in the commit history for +the affected files to find someone. + +We also use [GitLab Triage](https://gitlab.com/gitlab-org/gitlab-triage) to +automate some triaging policies. This is currently set up as a +[scheduled pipeline](https://gitlab.com/gitlab-org/quality/triage-ops/pipeline_schedules/10512/edit) +running on [quality/triage-ops](https://gitlab.com/gitlab-org/quality/triage-ops) project. + +## Labels + +To allow for asynchronous issue handling, we use [milestones](https://gitlab.com/groups/gitlab-org/-/milestones) and [labels](https://gitlab.com/gitlab-org/gitlab-ce/-/labels). Leads and product managers handle most of the scheduling into milestones. Labelling is a task for everyone. Most issues will have labels for at least one of the following: - Type: ~feature, ~bug, ~backstage, etc. -- Subject: ~wiki, ~"Container Registry", ~ldap, ~api, etc. -- Team: ~Documentation, ~Delivery, etc. - Stage: ~"devops::plan", ~"devops::create", etc. -- Group: ~"group::source code" ~"group::knowledge" ~"group::editor", etc. +- Group: ~"group::source code", ~"group::knowledge", ~"group::editor", etc. +- Category: ~"Category:Code Analytics", ~"Category:DevOps Score", ~"Category:Templates", etc. +- Feature: ~wiki, ~ldap, ~api, ~issues, ~"merge requests", etc. - Department: ~UX, ~Quality +- Team: ~Documentation, ~Delivery - Specialization: ~frontend, ~backend - Release Scoping: ~Deliverable, ~Stretch, ~"Next Patch Release" - Priority: ~P1, ~P2, ~P3, ~P4 @@ -23,14 +57,18 @@ All labels, their meaning and priority are defined on the If you come across an issue that has none of these, and you're allowed to set labels, you can _always_ add the team and type, and often also the subject. -[milestones-page]: https://gitlab.com/groups/gitlab-org/-/milestones - -## Type labels +### Type labels Type labels are very important. They define what kind of issue this is. Every -issue should have one or more. +issue should have one and only one. -Examples of type labels are ~feature, ~bug, ~backstage and ~security +The current type labels are: + +- ~feature +- ~bug +- ~backstage +- ~"support request" +- ~meta A number of type labels have a priority assigned to them, which automatically makes them float to the top, depending on their importance. @@ -38,115 +76,173 @@ makes them float to the top, depending on their importance. Type labels are always lowercase, and can have any color, besides blue (which is already reserved for subject labels). -The descriptions on the [labels page](https://gitlab.com/gitlab-org/gitlab-ce/-/labels) explain what falls under each type label. +The descriptions on the [labels page](https://gitlab.com/groups/gitlab-org/-/labels) +explain what falls under each type label. -## Subject labels +### Facet labels -Subject labels are labels that define what area or feature of GitLab this issue -hits. They are not always necessary, but very convenient. +Sometimes it's useful to refine the type of an issue. In those cases, you can +add facet labels. -Subject labels are now used to infer and apply relevant group and devops stage -labels. Please apply them whenever possible to facilitate accurate matching. -Please refer to [this merge request][inferred-labels] for more information. +Following is a non-exhaustive list of facet labels: -Examples of subject labels are ~wiki, ~ldap, ~api, -~issues, ~"merge requests", ~labels, and ~"Container Registry". +- ~enhancement: This label can refine an issue that has the ~feature label. +- ~"master:broken": This label can refine an issue that has the ~bug label. +- ~"master:flaky": This label can refine an issue that has the ~bug label. +- ~"technical debt": This label can refine an issue that has the ~backstage label. +- ~"static analysis": This label can refine an issue that has the ~backstage label. +- ~"ci-build": This label can refine an issue that has the ~backstage label. +- ~performance: A performance issue could describe a ~bug or a ~feature. +- ~security: A security issue could describe a ~bug or a ~feature. +- ~database: A database issue could describe a ~bug or a ~feature. +- ~customer: This relates to an issue that was created by a customer, or that is of interest for a customer. -If you are an expert in a particular area, it makes it easier to find issues to -work on. You can also subscribe to those labels to receive an email each time an -issue is labeled with a subject label corresponding to your expertise. +### Stage labels -Subject labels are always all-lowercase. +Stage labels specify which [stage](https://about.gitlab.com/handbook/product/categories/#hierarchy) the issue belongs to. -## Team labels +#### Naming and color convention -**Important**: Most of the team labels will be soon deprecated in favor of [Group labels](#group-labels). +Stage labels respects the `devops::<stage_key>` naming convention. +`<stage_key>` is the stage key as it is in the single source of truth for stages at +<https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml> +with `_` replaced with a space. -Team labels specify what team is responsible for this issue. -Assigning a team label makes sure issues get the attention of the appropriate -people. +For instance, the "Manage" stage is represented by the ~"devops::manage" label in +the `gitlab-org` group since its key under `stages` is `manage`. -The team labels planned for deprecation are: - -- ~Configure -- ~Create -- ~Defend -- ~Distribution -- ~Ecosystem -- ~Geo -- ~Gitaly -- ~Growth -- ~Manage -- ~Memory -- ~Monitor -- ~Plan -- ~Release -- ~Secure -- ~Verify - -The following team labels are **true** teams per our [organization structure](https://about.gitlab.com/company/team/structure/#organizational-structure) which will remain post deprecation. +The current stage labels can be found by [searching the labels list for `devops::`](https://gitlab.com/groups/gitlab-org/-/labels?search=devops::). -- ~Delivery -- ~Documentation - -The descriptions on the [labels page](https://gitlab.com/gitlab-org/gitlab-ce/-/labels) explain what falls under the -responsibility of each team. - -Team labels are always capitalized so that they show up as the first label for -any issue. - -## Stage labels +These labels are [scoped labels](../../user/project/labels.md#scoped-labels-premium) +and thus are mutually exclusive. -Stage labels specify which [DevOps stage][devops-stages] the issue belongs to. +The Stage labels are used to generate the [direction pages](https://about.gitlab.com/direction/) automatically. -The current stage labels can be found by [searching the labels list for `devops::`](https://gitlab.com/groups/gitlab-org/-/labels?search=devops%3A%3A). +### Group labels -These labels are [scoped labels](../../user/project/labels.md#scoped-labels-premium) -and thus are mutually exclusive. +Group labels specify which [groups](https://about.gitlab.com/company/team/structure/#product-groups) the issue belongs to. -The Stage labels are used to generate the [direction pages][direction-pages] automatically. +It's highly recommended to add a group label, as it's used by our triage +automation to +[infer the correct stage label](https://about.gitlab.com/handbook/engineering/quality/triage-operations/#auto-labelling-of-issues). -[devops-stages]: https://about.gitlab.com/direction/#devops-stages -[direction-pages]: https://about.gitlab.com/direction/ +#### Naming and color convention -## Group labels +Group labels respects the `group::<group_key>` naming convention and +their color is `#A8D695`. +`<group_key>` is the group key as it is in the single source of truth for groups at +<https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml>, +with `_` replaced with a space. -Group labels specify which [groups][structure-groups] the issue belongs to. +For instance, the "Continuous Integration" group is represented by the +~"group::continuous integration" label in the `gitlab-org` group since its key +under `stages.manage.groups` is `continuous_integration`. -The current group labels can be found by [searching the labels list for `group::`](https://gitlab.com/groups/gitlab-org/-/labels?search=group%3A%3A). +The current group labels can be found by [searching the labels list for `group::`](https://gitlab.com/groups/gitlab-org/-/labels?search=group::). These labels are [scoped labels](../../user/project/labels.md#scoped-labels-premium) and thus are mutually exclusive. -You can find the groups listed in the [Product Stages, Groups, and Categories][product-categories] page. +You can find the groups listed in the [Product Stages, Groups, and Categories](https://about.gitlab.com/handbook/product/categories/) page. -We use the term group to map down product requirements from our product stages. +We use the term group to map down product requirements from our product stages. As a team needs some way to collect the work their members are planning to be assigned to, we use the `~group::` labels to do so. Normally there is a 1:1 relationship between Stage labels and Group labels. In the spirit of "Everyone can contribute", any issue can be picked up by any group, depending on current priorities. For example, an issue labeled ~"devops::create" may be picked up by the ~"group::access" group. -We also use stage and group labels to help quantify our [throughput](https://about.gitlab.com/handbook/engineering/management/throughput). +We also use stage and group labels to help quantify our [throughput](https://about.gitlab.com/handbook/engineering/management/throughput). Please read [Stage and Group labels in Throughtput](https://about.gitlab.com/handbook/engineering/management/throughput/#stage-and-group-labels-in-throughput) for more information on how the labels are used in this context. -[structure-groups]: https://about.gitlab.com/company/team/structure/#groups -[product-categories]: https://about.gitlab.com/handbook/product/categories/ +### Category labels + +From the handbook's +[Product stages, groups, and categories](https://about.gitlab.com/handbook/product/categories/#hierarchy) +page: + +> Categories are high-level capabilities that may be a standalone product at +another company. e.g. Portfolio Management. + +It's highly recommended to add a category label, as it's used by our triage +automation to +[infer the correct group and stage labels](https://about.gitlab.com/handbook/engineering/quality/triage-operations/#auto-labelling-of-issues). + +If you are an expert in a particular area, it makes it easier to find issues to +work on. You can also subscribe to those labels to receive an email each time an +issue is labeled with a category label corresponding to your expertise. + +#### Naming and color convention + +Category labels respects the `Category:<Category Name>` naming convention and +their color is `#428BCA`. +`<Category Name>` is the category name as it is in the single source of truth for categories at +<https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/categories.yml>. + +For instance, the "Code Analytics" category is represented by the +~"Category:Code Analytics" label in the `gitlab-org` group since its +`code_analytics.name` value is "Code Analytics". + +If a category's label doesn't respect this naming convention, it should be specified +with [the `label` attribute](https://about.gitlab.com/handbook/marketing/website/#category-attributes) +in <https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/categories.yml>. + +### Feature labels + +From the handbook's +[Product stages, groups, and categories](https://about.gitlab.com/handbook/product/categories/#hierarchy) +page: + +> Features: Small, discrete functionalities. e.g. Issue weights. Some common +features are listed within parentheses to facilitate finding responsible PMs by keyword. + +It's highly recommended to add a feature label if no category label applies, as +it's used by our triage automation to +[infer the correct group and stage labels](https://about.gitlab.com/handbook/engineering/quality/triage-operations/#auto-labelling-of-issues). + +If you are an expert in a particular area, it makes it easier to find issues to +work on. You can also subscribe to those labels to receive an email each time an +issue is labeled with a feature label corresponding to your expertise. + +Examples of feature labels are ~wiki, ~ldap, ~api, ~issues, ~"merge requests" etc. + +#### Naming and color convention -## Department labels +Feature labels are all-lowercase. + +### Department labels The current department labels are: - ~UX - ~Quality -## Specialization labels +### Team labels + +**Important**: Most of the historical team labels (e.g. Manage, Plan etc.) are +now deprecated in favor of [Group labels](#group-labels) and [Stage labels](#stage-labels). + +Team labels specify what team is responsible for this issue. +Assigning a team label makes sure issues get the attention of the appropriate +people. + +The current team labels are: + +- ~Delivery +- ~Documentation + +#### Naming and color convention + +Team labels are always capitalized so that they show up as the first label for +any issue. + +### Specialization labels These labels narrow the [specialization](https://about.gitlab.com/company/team/structure/#specialist) on a unit of work. - ~frontend - ~backend -## Release Scoping labels +### Release scoping labels Release Scoping labels help us clearly communicate expectations of the work for the release. There are three levels of Release Scoping labels: @@ -164,7 +260,7 @@ Each issue scheduled for the current milestone should be labeled ~Deliverable or ~"Stretch". Any open issue for a previous milestone should be labeled ~"Next Patch Release", or otherwise rescheduled to a different milestone. -### Priority labels +#### Priority labels Priority labels help us define the time a ~bug fix should be completed. Priority determines how quickly the defect turnaround time must be. If there are multiple defects, the priority decides which defect has to be fixed immediately versus later. @@ -177,7 +273,7 @@ This label documents the planned timeline & urgency which is used to measure aga | ~P3 | Medium Priority | Within the next 3 releases (approx one quarter or 90 days) | | ~P4 | Low Priority | Anything outside the next 3 releases (more than one quarter or 120 days) | -## Severity labels +### Severity labels Severity labels help us clearly communicate the impact of a ~bug on users. There can be multiple facets of the impact. The below is a guideline. @@ -206,7 +302,7 @@ If a bug seems to fall between two severity labels, assign it to the higher-seve - Label colors are incorrect. - UI elements are not fully aligned. -## Label for community contributors +### Label for community contributors Issues that are beneficial to our users, 'nice to haves', that we currently do not have the capacity for or want to give the priority to, are labeled as @@ -236,11 +332,11 @@ After adding the ~"Accepting merge requests" label, we try to estimate the [weight](#issue-weight) of the issue. We use issue weight to let contributors know how difficult the issue is. Additionally: -- We advertise [`Accepting merge requests` issues with weight < 5][up-for-grabs] +- We advertise [`Accepting merge requests` issues with weight < 5](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight) as suitable for people that have never contributed to GitLab before on the [Up For Grabs campaign](http://up-for-grabs.net) - We encourage people that have never contributed to any open source project to - look for [`Accepting merge requests` issues with a weight of 1][first-timers] + look for [`Accepting merge requests` issues with a weight of 1](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight&weight=1) If you've decided that you would like to work on an issue, please @-mention the [appropriate product manager](https://about.gitlab.com/handbook/product/#who-to-talk-to-for-what) @@ -253,42 +349,29 @@ GitLab team members who apply the ~"Accepting merge requests" label to an issue should update the issue description with a responsible product manager, inviting any potential community contributor to @-mention per above. -[up-for-grabs]: https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight -[first-timers]: https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight&weight=1 +### Stewardship label -## Issue triaging - -Our issue triage policies are [described in our handbook]. You are very welcome -to help the GitLab team triage issues. We also organize [issue bash events] once -every quarter. - -The most important thing is making sure valid issues receive feedback from the -development team. Therefore the priority is mentioning developers that can help -on those issues. Please select someone with relevant experience from the -[GitLab team][team]. If there is nobody mentioned with that expertise look in -the commit history for the affected files to find someone. +For issues related to the open source stewardship of GitLab, +there is the ~"stewardship" label. -We also use [GitLab Triage] to automate some triaging policies. This is -currently set up as a [scheduled pipeline] running on [quality/triage-ops] -project. +This label is to be used for issues in which the stewardship of GitLab +is a topic of discussion. For instance if GitLab Inc. is planning to add +features from GitLab EE to GitLab CE, related issues would be labelled with +~"stewardship". -[described in our handbook]: https://about.gitlab.com/handbook/engineering/issue-triage/ -[issue bash events]: https://gitlab.com/gitlab-org/gitlab-ce/issues/17815 -[GitLab Triage]: https://gitlab.com/gitlab-org/gitlab-triage -[scheduled pipeline]: https://gitlab.com/gitlab-org/quality/triage-ops/pipeline_schedules/10512/edit -[quality/triage-ops]: https://gitlab.com/gitlab-org/quality/triage-ops -[team]: https://about.gitlab.com/team/ +A recent example of this was the issue for +[bringing the time tracking API to GitLab CE](https://gitlab.com/gitlab-org/gitlab-ce/issues/25517#note_20019084). ## Feature proposals To create a feature proposal for CE, open an issue on the -[issue tracker of CE][ce-tracker]. +[issue tracker of CE](https://gitlab.com/gitlab-org/gitlab-ce/issues). For feature proposals for EE, open an issue on the -[issue tracker of EE][ee-tracker]. +[issue tracker of EE](https://gitlab.com/gitlab-org/gitlab-ee/issues). In order to help track the feature proposals, we have created a -[`feature`][fl] label. For the time being, users that are not members +[`feature`](https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name=feature) label. For the time being, users that are not members of the project cannot add labels. You can instead ask one of the [core team](https://about.gitlab.com/community/core-team/) members to add the label ~feature to the issue or add the following code snippet right after your description in a new line: `~feature`. @@ -305,20 +388,6 @@ need to ask one of the [core team](https://about.gitlab.com/community/core-team/ If you want to create something yourself, consider opening an issue first to discuss whether it is interesting to include this in GitLab. -[fl]: https://gitlab.com/gitlab-org/gitlab-ce/issues?label_name=feature - -## Issue tracker guidelines - -**[Search the issue tracker][ce-tracker]** for similar entries before -submitting your own, there's a good chance somebody else had the same issue or -feature proposal. Show your support with an award emoji and/or join the -discussion. - -Please submit bugs using the ['Bug' issue template](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.gitlab/issue_templates/Bug.md) provided on the issue tracker. -The text in the parenthesis is there to help you with what to include. Omit it -when submitting the actual issue. You can copy-paste it and then edit as you -see fit. - ## Issue weight Issue weight allows us to get an idea of the amount of work required to solve @@ -364,7 +433,7 @@ addressed. ## Technical and UX debt In order to track things that can be improved in GitLab's codebase, -we use the ~"technical debt" label in [GitLab's issue tracker][ce-tracker]. +we use the ~"technical debt" label in [GitLab's issue tracker](https://gitlab.com/gitlab-org/gitlab-ce/issues). For missed user experience requirements, we use the ~"UX debt" label. These labels should be added to issues that describe things that can be improved, @@ -425,25 +494,6 @@ should be of the same quality as those created **must not** begin with `Follow-up`! The creating maintainer should also expect to be involved in some capacity when work begins on the follow-up issue. -## Stewardship - -For issues related to the open source stewardship of GitLab, -there is the ~"stewardship" label. - -This label is to be used for issues in which the stewardship of GitLab -is a topic of discussion. For instance if GitLab Inc. is planning to add -features from GitLab EE to GitLab CE, related issues would be labelled with -~"stewardship". - -A recent example of this was the issue for -[bringing the time tracking API to GitLab CE][time-tracking-issue]. - -[time-tracking-issue]: https://gitlab.com/gitlab-org/gitlab-ce/issues/25517#note_20019084 - --- [Return to Contributing documentation](index.md) - -[ce-tracker]: https://gitlab.com/gitlab-org/gitlab-ce/issues -[ee-tracker]: https://gitlab.com/gitlab-org/gitlab-ee/issues -[inferred-labels]: https://gitlab.com/gitlab-org/quality/triage-ops/merge_requests/155 diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 4e9c5c81379..bdb026d498d 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -1,4 +1,4 @@ -# Merge requests +# Merge requests workflow We welcome merge requests from everyone, with fixes and improvements to GitLab code, tests, and documentation. The issues that are specifically suitable diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index eb3b227473b..6c9fa983c96 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -9,7 +9,7 @@ An easy first step is to search for your error in Slack or google "GitLab (my er Available `RAILS_ENV` -- `production` (generally not for your main GDK db, but you may need this for e.g. omnibus) +- `production` (generally not for your main GDK db, but you may need this for e.g. Omnibus) - `development` (this is your main GDK db) - `test` (used for tests like rspec) diff --git a/doc/development/database_review.md b/doc/development/database_review.md index 3f1b359cb0b..367a481ee11 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -91,7 +91,7 @@ and details for a database reviewer: concurrent index/foreign key helpers (with transactions disabled) - Check consistency with `db/schema.rb` and that migrations are [reversible](migration_style_guide.md#reversibility) - Check queries timing (If any): Queries executed in a migration - need to fit comfortable within `15s` - preferably much less than that - on GitLab.com. + need to fit comfortably within `15s` - preferably much less than that - on GitLab.com. - Check [background migrations](background_migrations.md): - For data migrations, establish a time estimate for execution - They should only be used when migrating data in larger tables. diff --git a/doc/development/distributed_tracing.md b/doc/development/distributed_tracing.md index bfce7488a8d..4776c8348d4 100644 --- a/doc/development/distributed_tracing.md +++ b/doc/development/distributed_tracing.md @@ -179,4 +179,3 @@ By default, the Jaeger search UI is available at <http://localhost:16686/search> TIP: **Tip:** Don't forget that you will need to generate traces by using the application before they appear in the Jaeger UI. - diff --git a/doc/development/documentation/feature-change-workflow.md b/doc/development/documentation/feature-change-workflow.md index ac93ada5a4b..00c76fe0f1b 100644 --- a/doc/development/documentation/feature-change-workflow.md +++ b/doc/development/documentation/feature-change-workflow.md @@ -69,7 +69,7 @@ To follow a consistent workflow every month, documentation changes involve the Product Managers, the developer who shipped the feature, and the technical writer for the DevOps stage. Each role is described below. -The Documentation items in the GitLab CE/EE [Feature Proposal issue template](https://gitlab.com/gitlab-org/gitlab-ce/raw/template-improvements-for-documentation/.gitlab/issue_templates/Feature%20proposal.md) +The Documentation items in the GitLab CE/EE [Feature Proposal issue template](https://gitlab.com/gitlab-org/gitlab-ce/raw/master/.gitlab/issue_templates/Feature%20proposal.md) and default merge request template will assist you with following this process. ### Product Manager role diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index c9ae00d148a..edd83f67d3b 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -88,7 +88,7 @@ in an EE MR. To pass the test, simply remove the docs changes from the EE MR, an ## Changing document location Changing a document's location requires specific steps to be followed to ensure that -users can seamlessly access the new doc page, whether they are accesing content +users can seamlessly access the new doc page, whether they are accessing content on a GitLab instance domain at `/help` or at docs.gitlab.com. Be sure to ping a GitLab technical writer if you have any questions during the process (such as whether the move is necessary), and ensure that a technical writer reviews this diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index 025a946da0e..158e69df2a6 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -13,7 +13,7 @@ and the section on Content in the [Style Guide](styleguide.md). ## Components of a documentation page -Most pages will be dedicated to a specifig GitLab feature or to a use case that involves +Most pages will be dedicated to a specific GitLab feature or to a use case that involves one or more features, potentially in conjunction with third-party tools. Every feature or use case document should include the following content in the following sequence, diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index c1e3eb9680b..283e8bea8d5 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -506,15 +506,6 @@ Example: For more information, see the [confidential issue](../../user/project/issues/confidential_issues.md) `https://gitlab.com/gitlab-org/gitlab-ce/issues/<issue_number>`. ``` -### Unlinking emails - -By default, all email addresses will render in an email tag on docs.gitlab.com. -To escape the code block and unlink email addresses, use two backticks: - -```md -`` example@email.com `` -``` - ## Navigation To indicate the steps of navigation through the UI: @@ -783,8 +774,6 @@ For multiple paragraphs, use the symbol `>` before every line: > > - This is a list item > - Second item in the list -> -> ### This is an `h3` ``` Which renders to: @@ -795,9 +784,6 @@ Which renders to: > > - This is a list item > - Second item in the list -> -> ### This is an `h3` ->{:.no_toc} ## Terms diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 2217dedccd3..391361a4b8f 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -910,7 +910,7 @@ import bundle from 'ee_else_ce/protected_branches/protected_branches_bundle.js'; ``` See the frontend guide [performance section](fe_guide/performance.md) for -information on managing page-specific javascript within EE. +information on managing page-specific JavaScript within EE. ## Vue code in `assets/javascript` @@ -945,7 +945,7 @@ export default { - Since we [can't async load a mixin](https://github.com/vuejs/vue-loader/issues/418#issuecomment-254032223) we will use the [`ee_else_ce`](../development/ee_features.md#javascript-code-in-assetsjavascripts) alias we already have for webpack. - This means all the EE specific props, computed properties, methods, etc that are EE only should be in a mixin in the `ee/` folder and we need to create a CE counterpart of the mixin -##### Example: +##### Example ```javascript import mixin from 'ee_else_ce/path/mixin'; @@ -976,7 +976,7 @@ For regular JS files, the approach is similar. 1. An EE file should be created with the EE only code, and it should extend the CE counterpart. 1. For code inside functions that can't be extended, the code should be moved into a new file and we should use `ee_else_ce` helper: -#### Example: +#### Example ```javascript import eeCode from 'ee_else_ce/ee_code'; @@ -1047,8 +1047,6 @@ code base. Examples of backports include the following: Here is a workflow to make sure those changes end up backported safely into CE too. -(This approach does not refer to changes introduced via [csslab](https://gitlab.com/gitlab-org/csslab/).) - 1. **Make your changes in the EE branch.** If possible, keep a separated commit (to be squashed) to help backporting and review. 1. **Open merge request to EE project.** 1. **Apply the changes you made to CE files in a branch of the CE project.** (Tip: Use `patch` with the diff from your commit in EE branch) @@ -1057,7 +1055,7 @@ Here is a workflow to make sure those changes end up backported safely into CE t **Note:** regarding SCSS, make sure the files living outside `/ee/` don't diverge between CE and EE projects. -## gitlab-svgs +## GitLab-svgs Conflicts in `app/assets/images/icons.json` or `app/assets/images/icons.svg` can be resolved simply by regenerating those assets with diff --git a/doc/development/elasticsearch.md b/doc/development/elasticsearch.md index 635895051bc..f2412c249c1 100644 --- a/doc/development/elasticsearch.md +++ b/doc/development/elasticsearch.md @@ -40,9 +40,11 @@ There is no need to install any plugins If you're interested on working with the new beta repo indexer, all you need to do is: -- git clone git@gitlab.com:gitlab-org/gitlab-elasticsearch-indexer.git -- make -- make install +```sh +git clone git@gitlab.com:gitlab-org/gitlab-elasticsearch-indexer.git +make +make install +``` this adds `gitlab-elasticsearch-indexer` to `$GOPATH/bin`, please make sure that is in your `$PATH`. After that GitLab will find it and you'll be able to enable it in the admin settings area. @@ -148,26 +150,49 @@ Uses an [Edge NGram token filter](https://www.elastic.co/guide/en/elasticsearch/ - Searches can have their own analyzers. Remember to check when editing analyzers - `Character` filters (as opposed to token filters) always replace the original character, so they're not a good choice as they can hinder exact searches -## Architecture +## Zero downtime reindexing with multiple indices + +Currently GitLab can only handle a single version of setting. Any setting/schema changes would require reindexing everything from scratch. Since reindexing can take a long time, this can cause search functionality downtime. + +To avoid downtime, GitLab is working to support multiple indices that +can function at the same time. Whenever the schema changes, the admin +will be able to create a new index and reindex to it, while searches +continue to go to the older, stable index. Any data updates will be +forwarded to both indices. Once the new index is ready, an admin can +mark it active, which will direct all searches to it, and remove the old +index. + +This is also helpful for migrating to new servers, e.g. moving to/from AWS. + +Currently we are on the process of migrating to this new design. Everything is hardwired to work with one single version for now. -GitLab uses `elasticsearch-rails` for handling communication with Elasticsearch server. However, in order to achieve zero-downtime deployment during schema changes, an extra abstraction layer is built to allow: +### Architecture -* Indexing (writes) to multiple indexes, with different mappings -* Switching to different index for searches (reads) on the fly +The traditional setup, provided by `elasticsearch-rails`, is to communicate through its internal proxy classes. Developers would write model-specific logic in a module for the model to include in (e.g. `SnippetsSearch`). The `__elasticsearch__` methods would return a proxy object, e.g.: -Currently we are on the process of migrating models to this new design (e.g. `Snippet`), and it is hardwired to work with a single version for now. +- `Issue.__elasticsearch__` returns an instance of `Elasticsearch::Model::Proxy::ClassMethodsProxy` +- `Issue.first.__elasticsearch__` returns an instance of `Elasticsearch::Model::Proxy::InstanceMethodsProxy`. -Traditionally, `elasticsearch-rails` provides class and instance level `__elasticsearch__` proxy methods. If you call `Issue.__elasticsearch__`, you will get an instance of `Elasticsearch::Model::Proxy::ClassMethodsProxy`, and if you call `Issue.first.__elasticsearch__`, you will get an instance of `Elasticsearch::Model::Proxy::InstanceMethodsProxy`. These proxy objects would talk to Elasticsearch server directly. +These proxy objects would talk to Elasticsearch server directly (see top half of the diagram). -In the new design, `__elasticsearch__` instead represents one extra layer of proxy. It would keep multiple versions of the actual proxy objects, and it would forward read and write calls to the proxy of the intended version. + -The `elasticsearch-rails`'s way of specifying each model's mappings and other settings is to create a module for the model to include. However in the new design, each model would have its own corresponding subclassed proxy object, where the settings reside in. For example, snippet related setting in the past reside in `SnippetsSearch` module, but in the new design would reside in `SnippetClassProxy` (which is a subclass of `Elasticsearch::Model::Proxy::ClassMethodsProxy`). This reduces namespace pollution in model classes. +In the planned new design, each model would have a pair of corresponding subclassed proxy objects, in which model-specific logic is located. For example, `Snippet` would have `SnippetClassProxy` and `SnippetInstanceProxy` (being subclass of `Elasticsearch::Model::Proxy::ClassMethodsProxy` and `Elasticsearch::Model::Proxy::InstanceMethodsProxy`, respectively). + +`__elasticsearch__` would represent another layer of proxy object, keeping track of multiple actual proxy objects. It would forward method calls to the appropriate index. For example: + +- `model.__elasticsearch__.search` would be forwarded to the one stable index, since it is a read operation. +- `model.__elasticsearch__.update_document` would be forwarded to all indices, to keep all indices up-to-date. The global configurations per version are now in the `Elastic::(Version)::Config` class. You can change mappings there. ### Creating new version of schema -Currently GitLab would still work with a single version of setting. Once it is implemented, multiple versions of setting can exists in different folders (e.g. `ee/lib/elastic/v12p1` and `ee/lib/elastic/v12p3`). To keep a continuous git history, the latest version lives under the `/latest` folder, but is aliased as the latest version. +NOTE: **Note:** this is not applicable yet as multiple indices functionality is not fully implemented. + +Folders like `ee/lib/elastic/v12p1` contain snapshots of search logic from different versions. To keep a continuous Git history, the latest version lives under `ee/lib/elastic/latest`, but its classes are aliased under an actual version (e.g. `ee/lib/elastic/v12p3`). When referencing these classes, never use the `Latest` namespace directly, but use the actual version (e.g. `V12p3`). + +The version name basically follows GitLab's release version. If setting is changed in 12.3, we will create a new namespace called `V12p3` (p stands for "point"). Raise an issue if there is a need to name a version differently. If the current version is `v12p1`, and we need to create a new version for `v12p3`, the steps are as follows: @@ -176,7 +201,7 @@ If the current version is `v12p1`, and we need to create a new version for `v12p 1. Delete `v12p1` folder 1. Copy the entire folder of `latest` as `v12p1` 1. Change the namespace for files under `v12p1` folder from `Latest` to `V12p1` -1. Make changes to `Latest` as needed +1. Make changes to files under the `latest` folder as needed ## Troubleshooting diff --git a/doc/development/emails.md b/doc/development/emails.md index e6af075a282..5676c3b32f4 100644 --- a/doc/development/emails.md +++ b/doc/development/emails.md @@ -5,6 +5,10 @@ To view rendered emails "sent" in your development instance, visit [`/rails/letter_opener`](http://localhost:3000/rails/letter_opener). +Please note that [S/MIME signed](../administration/smime_signing_email.md) emails +[cannot be currently previewed](https://github.com/fgrehm/letter_opener_web/issues/96) with +`letter_opener`. + ## Mailer previews Rails provides a way to preview our mailer templates in HTML and plaintext using diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md index 09b4a3c3d96..6e7cf523f36 100644 --- a/doc/development/fe_guide/axios.md +++ b/doc/development/fe_guide/axios.md @@ -38,7 +38,7 @@ Advantages over [`spyOn()`]: - no need to create response objects - does not allow call through (which we want to avoid) -- simple API to test error cases +- simple API to test error cases - provides `replyOnce()` to allow for different responses We have also decided against using [axios interceptors] because they are not suitable for mocking. diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index 676bce32998..3a8ea04407f 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -81,7 +81,7 @@ bundle and included on the page. > can find this out by inspecting `document.body.dataset.page` within your > browser's developer console while on any page within gitlab. -#### Important Considerations: +#### Important Considerations - **Keep Entry Points Lite:** Page-specific javascript entry points should be as lite as possible. These diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index d3fa350b847..125b11afcd0 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -21,31 +21,31 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ 1. **Never Ever EVER** disable eslint globally for a file ```javascript - // bad - /* eslint-disable */ + // bad + /* eslint-disable */ - // better - /* eslint-disable some-rule, some-other-rule */ + // better + /* eslint-disable some-rule, some-other-rule */ - // best - // nothing :) + // best + // nothing :) ``` 1. If you do need to disable a rule for a single violation, try to do it as locally as possible ```javascript - // bad - /* eslint-disable no-new */ + // bad + /* eslint-disable no-new */ - import Foo from 'foo'; + import Foo from 'foo'; - new Foo(); + new Foo(); - // better - import Foo from 'foo'; + // better + import Foo from 'foo'; - // eslint-disable-next-line no-new - new Foo(); + // eslint-disable-next-line no-new + new Foo(); ``` 1. There are few rules that we need to disable due to technical debt. Which are: @@ -56,16 +56,16 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ followed by any global declarations, then a blank newline prior to any imports or code. ```javascript - // bad - /* global Foo */ - /* eslint-disable no-new */ - import Bar from './bar'; + // bad + /* global Foo */ + /* eslint-disable no-new */ + import Bar from './bar'; - // good - /* eslint-disable no-new */ - /* global Foo */ + // good + /* eslint-disable no-new */ + /* global Foo */ - import Bar from './bar'; + import Bar from './bar'; ``` 1. **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead. @@ -73,23 +73,23 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ 1. When declaring multiple globals, always use one `/* global [name] */` line per variable. ```javascript - // bad - /* globals Flash, Cookies, jQuery */ + // bad + /* globals Flash, Cookies, jQuery */ - // good - /* global Flash */ - /* global Cookies */ - /* global jQuery */ + // good + /* global Flash */ + /* global Cookies */ + /* global jQuery */ ``` 1. Use up to 3 parameters for a function or class. If you need more accept an Object instead. ```javascript - // bad - fn(p1, p2, p3, p4) {} + // bad + fn(p1, p2, p3, p4) {} - // good - fn(options) {} + // good + fn(options) {} ``` #### Modules, Imports, and Exports @@ -97,32 +97,32 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ 1. Use ES module syntax to import modules ```javascript - // bad - const SomeClass = require('some_class'); + // bad + const SomeClass = require('some_class'); - // good - import SomeClass from 'some_class'; + // good + import SomeClass from 'some_class'; - // bad - module.exports = SomeClass; + // bad + module.exports = SomeClass; - // good - export default SomeClass; + // good + export default SomeClass; ``` Import statements are following usual naming guidelines, for example object literals use camel case: ```javascript - // some_object file - export default { - key: 'value', - }; + // some_object file + export default { + key: 'value', + }; - // bad - import ObjectLiteral from 'some_object'; + // bad + import ObjectLiteral from 'some_object'; - // good - import objectLiteral from 'some_object'; + // good + import objectLiteral from 'some_object'; ``` 1. Relative paths: when importing a module in the same directory, a child @@ -171,22 +171,22 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ 1. Avoid adding to the global namespace. ```javascript - // bad - window.MyClass = class { /* ... */ }; + // bad + window.MyClass = class { /* ... */ }; - // good - export default class MyClass { /* ... */ } + // good + export default class MyClass { /* ... */ } ``` 1. Side effects are forbidden in any script which contains export ```javascript - // bad - export default class MyClass { /* ... */ } + // bad + export default class MyClass { /* ... */ } - document.addEventListener("DOMContentLoaded", function(event) { - new MyClass(); - } + document.addEventListener("DOMContentLoaded", function(event) { + new MyClass(); + } ``` #### Data Mutation and Pure functions @@ -257,17 +257,17 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ `.reduce` or `.filter` ```javascript - const users = [ { name: 'Foo' }, { name: 'Bar' } ]; + const users = [ { name: 'Foo' }, { name: 'Bar' } ]; - // bad - users.forEach((user, index) => { - user.id = index; - }); + // bad + users.forEach((user, index) => { + user.id = index; + }); - // good - const usersWithId = users.map((user, index) => { - return Object.assign({}, user, { id: index }); - }); + // good + const usersWithId = users.map((user, index) => { + return Object.assign({}, user, { id: index }); + }); ``` #### Parse Strings into Numbers @@ -275,14 +275,14 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ 1. `parseInt()` is preferable over `Number()` or `+` ```javascript - // bad - +'10' // 10 + // bad + +'10' // 10 - // good - Number('10') // 10 + // good + Number('10') // 10 - // better - parseInt('10', 10); + // better + parseInt('10', 10); ``` #### CSS classes used for JavaScript @@ -290,15 +290,15 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/ 1. If the class is being used in Javascript it needs to be prepend with `js-` ```html - // bad - <button class="add-user"> - Add User - </button> + // bad + <button class="add-user"> + Add User + </button> - // good - <button class="js-add-user"> - Add User - </button> + // good + <button class="js-add-user"> + Add User + </button> ``` ### Vue.js @@ -315,41 +315,41 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. Use a function in the bundle file to instantiate the Vue component: ```javascript - // bad - class { - init() { - new Component({}) - } + // bad + class { + init() { + new Component({}) } + } - // good - document.addEventListener('DOMContentLoaded', () => new Vue({ - el: '#element', - components: { - componentName - }, - render: createElement => createElement('component-name'), - })); + // good + document.addEventListener('DOMContentLoaded', () => new Vue({ + el: '#element', + components: { + componentName + }, + render: createElement => createElement('component-name'), + })); ``` 1. Do not use a singleton for the service or the store ```javascript - // bad - class Store { - constructor() { - if (!this.prototype.singleton) { - // do something - } + // bad + class Store { + constructor() { + if (!this.prototype.singleton) { + // do something } } + } - // good - class Store { - constructor() { - // do something - } + // good + class Store { + constructor() { + // do something } + } ``` 1. Use `.vue` for Vue templates. Do not use `%template` in HAML. @@ -360,36 +360,36 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. **Reference Naming**: Use PascalCase for their instances: ```javascript - // bad - import cardBoard from 'cardBoard.vue' + // bad + import cardBoard from 'cardBoard.vue' - components: { - cardBoard, - }; + components: { + cardBoard, + }; - // good - import CardBoard from 'cardBoard.vue' + // good + import CardBoard from 'cardBoard.vue' - components: { - CardBoard, - }; + components: { + CardBoard, + }; ``` 1. **Props Naming:** Avoid using DOM component prop names. 1. **Props Naming:** Use kebab-case instead of camelCase to provide props in templates. ```javascript - // bad - <component class="btn"> + // bad + <component class="btn"> - // good - <component css-class="btn"> + // good + <component css-class="btn"> - // bad - <component myProp="prop" /> + // bad + <component myProp="prop" /> - // good - <component my-prop="prop" /> + // good + <component my-prop="prop" /> ``` [#34371]: https://gitlab.com/gitlab-org/gitlab-ce/issues/34371 @@ -401,37 +401,37 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. With more than one attribute, all attributes should be on a new line: ```javascript - // bad - <component v-if="bar" - param="baz" /> + // bad + <component v-if="bar" + param="baz" /> - <button class="btn">Click me</button> + <button class="btn">Click me</button> - // good - <component - v-if="bar" - param="baz" - /> + // good + <component + v-if="bar" + param="baz" + /> - <button class="btn"> - Click me - </button> + <button class="btn"> + Click me + </button> ``` 1. The tag can be inline if there is only one attribute: ```javascript - // good - <component bar="bar" /> + // good + <component bar="bar" /> - // good - <component - bar="bar" - /> + // good + <component + bar="bar" + /> - // bad - <component - bar="bar" /> + // bad + <component + bar="bar" /> ``` #### Quotes @@ -439,15 +439,15 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. Always use double quotes `"` inside templates and single quotes `'` for all other JS. ```javascript - // bad - template: ` - <button :class='style'>Button</button> - ` + // bad + template: ` + <button :class='style'>Button</button> + ` - // good - template: ` - <button :class="style">Button</button> - ` + // good + template: ` + <button :class="style">Button</button> + ` ``` #### Props @@ -455,37 +455,37 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. Props should be declared as an object ```javascript - // bad - props: ['foo'] - - // good - props: { - foo: { - type: String, - required: false, - default: 'bar' - } + // bad + props: ['foo'] + + // good + props: { + foo: { + type: String, + required: false, + default: 'bar' } + } ``` 1. Required key should always be provided when declaring a prop ```javascript - // bad - props: { - foo: { - type: String, - } + // bad + props: { + foo: { + type: String, } + } - // good - props: { - foo: { - type: String, - required: false, - default: 'bar' - } + // good + props: { + foo: { + type: String, + required: false, + default: 'bar' } + } ``` 1. Default key should be provided if the prop is not required. @@ -493,30 +493,30 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. On those a default key should not be provided. ```javascript - // good - props: { - foo: { - type: String, - required: false, - } + // good + props: { + foo: { + type: String, + required: false, } + } - // good - props: { - foo: { - type: String, - required: false, - default: 'bar' - } + // good + props: { + foo: { + type: String, + required: false, + default: 'bar' } + } - // good - props: { - foo: { - type: String, - required: true - } + // good + props: { + foo: { + type: String, + required: true } + } ``` #### Data @@ -524,17 +524,17 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. `data` method should always be a function ```javascript - // bad - data: { - foo: 'foo' - } + // bad + data: { + foo: 'foo' + } - // good - data() { - return { - foo: 'foo' - }; - } + // good + data() { + return { + foo: 'foo' + }; + } ``` #### Directives @@ -542,31 +542,31 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. Shorthand `@` is preferable over `v-on` ```javascript - // bad - <component v-on:click="eventHandler"/> + // bad + <component v-on:click="eventHandler"/> - // good - <component @click="eventHandler"/> + // good + <component @click="eventHandler"/> ``` 1. Shorthand `:` is preferable over `v-bind` ```javascript - // bad - <component v-bind:class="btn"/> + // bad + <component v-bind:class="btn"/> - // good - <component :class="btn"/> + // good + <component :class="btn"/> ``` 1. Shorthand `#` is preferable over `v-slot` ```javascript - // bad - <template v-slot:header></template> + // bad + <template v-slot:header></template> - // good - <template #header></template> + // good + <template #header></template> ``` #### Closing tags @@ -574,11 +574,11 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. 1. Prefer self closing component tags ```javascript - // bad - <component></component> + // bad + <component></component> - // good - <component /> + // good + <component /> ``` #### Ordering @@ -610,48 +610,48 @@ When using `v-for` you need to provide a *unique* `:key` attribute for each item 1. If the elements of the array being iterated have an unique `id` it is advised to use it: ```html - <div - v-for="item in items" - :key="item.id" - > - <!-- content --> - </div> + <div + v-for="item in items" + :key="item.id" + > + <!-- content --> + </div> ``` 1. When the elements being iterated don't have a unique id, you can use the array index as the `:key` attribute ```html - <div - v-for="(item, index) in items" - :key="index" - > - <!-- content --> - </div> + <div + v-for="(item, index) in items" + :key="index" + > + <!-- content --> + </div> ``` 1. When using `v-for` with `template` and there is more than one child element, the `:key` values must be unique. It's advised to use `kebab-case` namespaces. ```html - <template v-for="(item, index) in items"> - <span :key="`span-${index}`"></span> - <button :key="`button-${index}`"></button> - </template> + <template v-for="(item, index) in items"> + <span :key="`span-${index}`"></span> + <button :key="`button-${index}`"></button> + </template> ``` 1. When dealing with nested `v-for` use the same guidelines as above. ```html - <div - v-for="item in items" - :key="item.id" + <div + v-for="item in items" + :key="item.id" + > + <span + v-for="element in array" + :key="element.id" > - <span - v-for="element in array" - :key="element.id" - > - <!-- content --> - </span> - </div> + <!-- content --> + </span> + </div> ``` Useful links: @@ -664,19 +664,19 @@ Useful links: 1. Tooltips: Do not rely on `has-tooltip` class name for Vue components ```javascript - // bad - <span - class="has-tooltip" - title="Some tooltip text"> - Text - </span> + // bad + <span + class="has-tooltip" + title="Some tooltip text"> + Text + </span> - // good - <span - v-tooltip - title="Some tooltip text"> - Text - </span> + // good + <span + v-tooltip + title="Some tooltip text"> + Text + </span> ``` 1. Tooltips: When using a tooltip, include the tooltip directive, `./app/assets/javascripts/vue_shared/directives/tooltip.js` @@ -684,13 +684,13 @@ Useful links: 1. Don't change `data-original-title`. ```javascript - // bad - <span data-original-title="tooltip text">Foo</span> + // bad + <span data-original-title="tooltip text">Foo</span> - // good - <span title="tooltip text">Foo</span> + // good + <span title="tooltip text">Foo</span> - $('span').tooltip('_fixTitle'); + $('span').tooltip('_fixTitle'); ``` ### The Javascript/Vue Accord diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index 9eeaee4482f..557d3132d71 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -313,7 +313,7 @@ export default { 1. Do not call a mutation directly. Always use an action to commit a mutation. Doing so will keep consistency throughout the application. From Vuex docs: - > why don't we just call store.commit('action') directly? Well, remember that mutations must be synchronous? Actions aren't. We can perform asynchronous operations inside an action. + > Why don't we just call store.commit('action') directly? Well, remember that mutations must be synchronous? Actions aren't. We can perform asynchronous operations inside an action. ```javascript // component.vue diff --git a/doc/development/feature_flags/index.md b/doc/development/feature_flags/index.md index 56872f8c075..f1374b9e280 100644 --- a/doc/development/feature_flags/index.md +++ b/doc/development/feature_flags/index.md @@ -8,5 +8,5 @@ disable those changes, without having to revert an entire release. Before using feature flags for GitLab's development, read through the following: - [Process for using features flags](process.md). -- [Developing with feature flags documentation](development.md). -- [Controlling feature flags documentation](controls.md). +- [Developing with feature flags](development.md). +- [Controlling feature flags](controls.md). diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md index 475d1c1611e..44af2b020a4 100644 --- a/doc/development/file_storage.md +++ b/doc/development/file_storage.md @@ -2,6 +2,8 @@ We use the [CarrierWave] gem to handle file upload, store and retrieval. +File uploads should be accelerated by workhorse, for details please refer to [uploads development documentation](uploads.md). + There are many places where file uploading is used, according to contexts: - System diff --git a/doc/development/filtering_by_label.md b/doc/development/filtering_by_label.md index 5e7376db725..dd8944ff1c8 100644 --- a/doc/development/filtering_by_label.md +++ b/doc/development/filtering_by_label.md @@ -40,16 +40,14 @@ In particular, note that: This is more complicated than is ideal. It makes the query construction more prone to errors (such as -[gitlab-org/gitlab-ce#15557](https://gitlab.com/gitlab-org/gitlab-ce/issues/15557)). +[issue #15557](https://gitlab.com/gitlab-org/gitlab-ce/issues/15557)). ## Attempt A: WHERE EXISTS ### Attempt A1: use multiple subqueries with WHERE EXISTS -In -[gitlab-org/gitlab-ce#37137](https://gitlab.com/gitlab-org/gitlab-ce/issues/37137) -and its associated merge request -[gitlab-org/gitlab-ce!14022](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14022), +In [issue #37137](https://gitlab.com/gitlab-org/gitlab-ce/issues/37137) +and its associated [merge request](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14022), we tried to replace the `GROUP BY` with multiple uses of `WHERE EXISTS`. For the example above, this would give: @@ -81,12 +79,11 @@ it did not improve query performance. ## Attempt B: Denormalize using an array column -Having [removed MySQL support in GitLab -12.1](https://about.gitlab.com/2019/06/27/removing-mysql-support/), using -[Postgres's arrays](https://www.postgresql.org/docs/9.6/arrays.html) became more +Having [removed MySQL support in GitLab 12.1](https://about.gitlab.com/2019/06/27/removing-mysql-support/), +using [Postgres's arrays](https://www.postgresql.org/docs/9.6/arrays.html) became more tractable as we didn't have to support two databases. We discussed denormalizing the `label_links` table for querying in -[gitlab-org/gitlab-ce#49651](https://gitlab.com/gitlab-org/gitlab-ce/issues/49651), +[issue #49651](https://gitlab.com/gitlab-org/gitlab-ce/issues/49651), with two options: label IDs and titles. We can think of both of those as array columns on `issues`, `merge_requests`, @@ -150,8 +147,7 @@ WHERE label_titles @> ARRAY['Plan', 'backend'] ``` -And our [tests in -gitlab-org/gitlab-ce#49651](https://gitlab.com/gitlab-org/gitlab-ce/issues/49651#note_188777346) +And our [tests in issue #49651](https://gitlab.com/gitlab-org/gitlab-ce/issues/49651#note_188777346) showed that this could be fast. However, at present, the disadvantages outweigh the advantages. diff --git a/doc/development/gemfile.md b/doc/development/gemfile.md index ec9718cea71..8d93c52e7bc 100644 --- a/doc/development/gemfile.md +++ b/doc/development/gemfile.md @@ -3,9 +3,9 @@ When adding a new entry to `Gemfile` or upgrading an existing dependency pay attention to the following rules. -## No gems fetched from git repositories +## No gems fetched from Git repositories -We do not allow gems that are fetched from git repositories. All gems have +We do not allow gems that are fetched from Git repositories. All gems have to be available in the RubyGems index. We want to minimize external build dependencies and build times. diff --git a/doc/development/geo.md b/doc/development/geo.md index 24f16eae9fa..cc3e2d1ccc5 100644 --- a/doc/development/geo.md +++ b/doc/development/geo.md @@ -170,7 +170,7 @@ while `pull` requests will continue to be served by the **secondary** node for m HTTPS and SSH requests are handled differently: - With HTTPS, we will give the user a `HTTP 302 Redirect` pointing to the project on the **primary** node. - The git client is wise enough to understand that status code and process the redirection. + The Git client is wise enough to understand that status code and process the redirection. - With SSH, because there is no equivalent way to perform a redirect, we have to proxy the request. This is done inside [`gitlab-shell`](https://gitlab.com/gitlab-org/gitlab-shell), by first translating the request to the HTTP protocol, and then proxying it to the **primary** node. diff --git a/doc/development/git_object_deduplication.md b/doc/development/git_object_deduplication.md index 5ce59891afa..e8af6346524 100644 --- a/doc/development/git_object_deduplication.md +++ b/doc/development/git_object_deduplication.md @@ -8,30 +8,6 @@ storage disk use. To counteract this problem, we are adding Git object deduplication for forks to GitLab. In this document, we will describe how GitLab implements Git object deduplication. -## Enabling Git object deduplication via feature flags - -As of GitLab 12.0, Git object deduplication in GitLab is still behind a -feature flag. In this document, you can read about the effects of -enabling the feature. Also, note that Git object deduplication is -limited to forks of public projects on hashed repository storage. - -You can enable deduplication globally by setting the `object_pools` -feature flag to `true`: - -``` {.ruby} -Feature.enable(:object_pools) -``` - -Or just for forks of a specific project: - -``` {.ruby} -fork_parent = Project.find(MY_PROJECT_ID) -Feature.enable(:object_pools, fork_parent) -``` - -To check if a project uses Git object deduplication, look in a Rails -console if `project.pool_repository` is present. - ## Pool repositories ### Understanding Git alternates @@ -193,7 +169,7 @@ There are three different things that can go wrong here. In this case, we miss out on disk space savings but all RPC's on A itself will function fine. The next time garbage collection runs on A, the alternates connection gets established in Gitaly. This is done by -`Projects::GitDeduplicationService` in gitlab-rails. +`Projects::GitDeduplicationService` in GitLab Rails. #### 2. SQL says repo A belongs to pool P1 but Gitaly says A has alternate objects in pool P2 @@ -210,7 +186,7 @@ When a pool repository record is created in SQL on a Geo primary, this will eventually trigger an event on the Geo secondary. The Geo secondary will then create the pool repository in Gitaly. This leads to an "eventually consistent" situation because as each pool participant gets -synchronized, Geo will eventuall trigger garbage collection in Gitaly on +synchronized, Geo will eventually trigger garbage collection in Gitaly on the secondary, at which stage Git objects will get deduplicated. > TODO How do we handle the edge case where at the time the Geo diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index 2ade59b76ed..592fc13873b 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -45,13 +45,13 @@ The process for adding new Gitaly features is: - release a new version of gitaly-proto - write implementation and tests for the RPC [in Gitaly](https://gitlab.com/gitlab-org/gitaly), in Go or Ruby - release a new version of Gitaly -- write client code in gitlab-ce/ee, gitlab-workhorse or gitlab-shell that calls the new Gitaly RPC +- write client code in GitLab CE/EE, GitLab Workhorse or GitLab Shell that calls the new Gitaly RPC These steps often overlap. It is possible to use an unreleased version of Gitaly and gitaly-proto during testing and development. - See the [Gitaly repo](https://gitlab.com/gitlab-org/gitaly/blob/master/CONTRIBUTING.md#development-and-testing-with-a-custom-gitaly-proto) for instructions on writing server side code with an unreleased protocol. -- See [below](#running-tests-with-a-locally-modified-version-of-gitaly) for instructions on running gitlab-ce tests with a modified version of Gitaly. +- See [below](#running-tests-with-a-locally-modified-version-of-gitaly) for instructions on running GitLab CE tests with a modified version of Gitaly. - In GDK run `gdk install` and restart `gdk run` (or `gdk run app`) to use a locally modified Gitaly version for development ### Gitaly-ruby @@ -146,7 +146,7 @@ Once the code is wrapped in this block, this code-path will be excluded from n+1 ## Request counts -Commits and other git data, is now fetched through Gitaly. These fetches can, +Commits and other Git data, is now fetched through Gitaly. These fetches can, much like with a database, be batched. This improves performance for the client and for Gitaly itself and therefore for the users too. To keep performance stable and guard performance regressions, Gitaly calls can be counted and the call count @@ -164,10 +164,10 @@ end ## Running tests with a locally modified version of Gitaly -Normally, gitlab-ce/ee tests use a local clone of Gitaly in +Normally, GitLab CE/EE tests use a local clone of Gitaly in `tmp/tests/gitaly` pinned at the version specified in `GITALY_SERVER_VERSION`. The `GITALY_SERVER_VERSION` file supports -`=my-branch` syntax to use a custom branch in gitlab-org/gitaly. If +`=my-branch` syntax to use a custom branch in <https://gitlab.com/gitlab-org/gitaly>. If you want to run tests locally against a modified version of Gitaly you can replace `tmp/tests/gitaly` with a symlink. This is much faster because the `=my-branch` syntax forces a Gitaly re-install each time @@ -276,9 +276,9 @@ Here are the steps to gate a new feature in Gitaly behind a feature flag. require.NoError(t, err) ``` -### Gitlab-Rails +### GitLab Rails -1. Add feature flag to `lib/gitlab/gitaly_client.rb` (in gitlab-rails): +1. Add feature flag to `lib/gitlab/gitaly_client.rb` (in GitLab Rails): ```ruby SERVER_FEATURE_FLAGS = %w[go-find-all-tags].freeze diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index 83444093f9c..2df0e846671 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -94,7 +94,7 @@ become available, you will be able to share job templates like this Dependencies should be kept to the minimum. The introduction of a new dependency should be argued in the merge request, as per our [Approval Guidelines](../code_review.md#approval-guidelines). Both [License -Management](../../user/project/merge_requests/license_management.md) +Management](../../user/application_security/license_compliance/index.md) **(ULTIMATE)** and [Dependency Scanning](../../user/application_security/dependency_scanning/index.md) **(ULTIMATE)** should be activated on all projects to ensure new dependencies diff --git a/doc/development/hash_indexes.md b/doc/development/hash_indexes.md index e6c1b3590b1..417ea18e22f 100644 --- a/doc/development/hash_indexes.md +++ b/doc/development/hash_indexes.md @@ -1,6 +1,6 @@ # Hash Indexes -Both PostgreSQL and MySQL support hash indexes besides the regular btree +PostgreSQL supports hash indexes besides the regular btree indexes. Hash indexes however are to be avoided at all costs. While they may _sometimes_ provide better performance the cost of rehashing can be very high. More importantly: at least until PostgreSQL 10.0 hash indexes are not diff --git a/doc/development/i18n/translation.md b/doc/development/i18n/translation.md index 62be3786549..15b1af1aa8f 100644 --- a/doc/development/i18n/translation.md +++ b/doc/development/i18n/translation.md @@ -92,4 +92,3 @@ To propose additions to the glossary please In French, the "écriture inclusive" is now over (see on [Legifrance](https://www.legifrance.gouv.fr/affichTexte.do?cidTexte=JORFTEXT000036068906&categorieLien=id)). So, to include both genders, write “Utilisateurs et utilisatrices” instead of “Utilisateur·rice·s”. When space is missing, the male gender should be used alone. - diff --git a/doc/development/img/elasticsearch_architecture.svg b/doc/development/img/elasticsearch_architecture.svg new file mode 100644 index 00000000000..2f38f9b04ee --- /dev/null +++ b/doc/development/img/elasticsearch_architecture.svg @@ -0,0 +1 @@ +<svg version="1.2" width="210mm" height="297mm" viewBox="0 0 21000 29700" preserveAspectRatio="xMidYMid" fill-rule="evenodd" stroke-width="28.222" stroke-linejoin="round" xmlns="http://www.w3.org/2000/svg"><defs class="ClipPathGroup"><clipPath id="a" clipPathUnits="userSpaceOnUse"><path d="M0 0h21000v29700H0z"/></clipPath></defs><g class="SlideGroup"><g class="Slide" clip-path="url(#a)"><g class="Page"><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1975 5575h3051v1651H1975z"/><path fill="#FFF" d="M3500 7200H2000V5600h3000v1600H3500z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M3500 7200H2000V5600h3000v1600H3500z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="2778" y="6311"><tspan>Snippet</tspan></tspan><tspan class="TextPosition" x="2099" y="6785"><tspan>(ActiveRecord)</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1475 3975h4051v3551H1475z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M3500 7500H1500V4000h4000v3500H3500z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="1788" y="5048"><tspan>ApplicationSearch</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M5975 4675h8051v701H5975z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M6000 5350h4000v-650h4000"/></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M5975 5325h8051v1101H5975z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M6000 5350h4000v1050h4000"/></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1075 2875h4951v4951H1075z"/><path fill="none" stroke="#F33" stroke-width="50" d="M3550 7800H1100V2900h4900v4900H3550z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="700"><tspan class="TextPosition" x="1946" y="3514"><tspan fill="#C9211E">SnippetsSearch</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1975 12175h3051v1651H1975z"/><path fill="#FFF" d="M3500 13800H2000v-1600h3000v1600H3500z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M3500 13800H2000v-1600h3000v1600H3500z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="2778" y="12911"><tspan>Snippet</tspan></tspan><tspan class="TextPosition" x="2099" y="13385"><tspan>(ActiveRecord)</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1075 10775h4951v3251H1075z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M3550 14000H1100v-3200h4900v3200H3550z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="2511" y="11461"><tspan>Application</tspan></tspan><tspan class="TextPosition" x="1933" y="11935"><tspan>VersionedSearch</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M3525 13975h4501v7451H3525z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M3550 14000v7400h4450"/></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M14008 14075h4985v851h-4985z"/><path fill="none" stroke="#999" stroke-width="50" d="M16500 14900h-2467v-800h4934v800h-2467z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="14720" y="14648"><tspan fill="gray">ClassMethodProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M13375 13075h6251v2151h-6251z"/><path fill="none" stroke="#F33" stroke-width="50" d="M16500 15200h-3100v-2100h6200v2100h-3100z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="700"><tspan class="TextPosition" x="13799" y="13731"><tspan fill="#C9211E">V12p1::SnippetClassProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M7975 14575h3051v1851H7975z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M9500 16400H8000v-1800h3000v1800H9500z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="8277" y="15411"><tspan>MultiVersion-</tspan></tspan><tspan class="TextPosition" x="8429" y="15885"><tspan>ClassProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M14008 16875h4985v851h-4985z"/><path fill="none" stroke="#999" stroke-width="50" d="M16500 17700h-2467v-800h4934v800h-2467z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="14720" y="17448"><tspan fill="gray">ClassMethodProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M13375 15875h6251v2151h-6251z"/><path fill="none" stroke="#F33" stroke-width="50" d="M16500 18000h-3100v-2100h6200v2100h-3100z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="700"><tspan class="TextPosition" x="13799" y="16531"><tspan fill="#C9211E">V12p2::SnippetClassProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M10975 14125h2451v1401h-2451z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M11000 15500h1463v-1350h937"/></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M10975 15475h2451v1501h-2451z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M11000 15500h1463v1450h937"/></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M3525 13975h4501v1551H3525z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M3550 14000v1500h4450"/></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M14008 19975h4985v851h-4985z"/><path fill="none" stroke="#999" stroke-width="50" d="M16500 20800h-2467v-800h4934v800h-2467z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="14445" y="20548"><tspan fill="gray">InstanceMethodProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M13375 18975h6251v2151h-6251z"/><path fill="none" stroke="#F33" stroke-width="50" d="M16500 21100h-3100v-2100h6200v2100h-3100z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="700"><tspan class="TextPosition" x="13505" y="19631"><tspan fill="#C9211E">V12p1::SnippetInstanceProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M7975 20275h3051v2251H7975z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M9500 22500H8000v-2200h3000v2200H9500z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="8277" y="21311"><tspan>MultiVersion-</tspan></tspan><tspan class="TextPosition" x="8154" y="21785"><tspan>InstanceProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M14008 22775h4985v851h-4985z"/><path fill="none" stroke="#999" stroke-width="50" d="M16500 23600h-2467v-800h4934v800h-2467z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="14445" y="23348"><tspan fill="gray">InstanceMethodProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M13375 21775h6251v2151h-6251z"/><path fill="none" stroke="#F33" stroke-width="50" d="M16500 23900h-3100v-2100h6200v2100h-3100z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="700"><tspan class="TextPosition" x="13505" y="22431"><tspan fill="#C9211E">V12p2::SnippetInstanceProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M10975 20025h2451v1401h-2451z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M11000 21400h1463v-1350h937"/></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M10975 21375h2451v1501h-2451z"/><path fill="none" stroke="#3465A4" stroke-width="50" d="M11000 21400h1463v1450h937"/></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M900 1600h10697v879H900z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="564" font-weight="400"><tspan class="TextPosition" x="1150" y="2233"><tspan>Standard elasticsearch-rails setup</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M900 9300h7683v879H900z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="564" font-weight="400"><tspan class="TextPosition" x="1150" y="9933"><tspan>GitLab multi-indices setup</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M3400 21300h4821v1197H3400z"/><text class="TextShape"><tspan class="TextParagraph" font-size="388" font-weight="400"><tspan class="TextPosition" x="4250" y="21840"><tspan fill="gray">(instance method)</tspan></tspan><tspan class="TextPosition" x="3651" y="22264"><tspan font-family="Courier" font-size="423">__elasticsearch__</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M3380 15400h4821v1197H3380z"/><text class="TextShape"><tspan class="TextParagraph" font-size="388" font-weight="400"><tspan class="TextPosition" x="4512" y="15940"><tspan fill="gray">(class method)</tspan></tspan><tspan class="TextPosition" x="3631" y="16364"><tspan font-family="Courier" font-size="423">__elasticsearch__</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M9000 3500h4821v1197H9000z"/><text class="TextShape"><tspan class="TextParagraph" font-size="388" font-weight="400"><tspan class="TextPosition" x="10132" y="4040"><tspan fill="gray">(class method)</tspan></tspan><tspan class="TextPosition" x="9251" y="4464"><tspan font-family="Courier" font-size="423">__elasticsearch__</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M9000 6400h4821v1197H9000z"/><text class="TextShape"><tspan class="TextParagraph" font-size="388" font-weight="400"><tspan class="TextPosition" x="9850" y="6940"><tspan fill="gray">(instance method)</tspan></tspan><tspan class="TextPosition" x="9251" y="7364"><tspan font-family="Courier" font-size="423">__elasticsearch__</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1975 25175h2051v851H1975z"/><path fill="none" stroke="#999" stroke-width="50" d="M3000 26000H2000v-800h2000v800H3000z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="2634" y="25748"><tspan fill="gray">Foo</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M4400 25200h7101v726H4400z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="4650" y="25710"><tspan>elasticsearch-rails’ internal class</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M4400 26400h8601v1200H4400z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="4650" y="26910"><tspan>where model-specific logic is</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M1975 26275h2051v851H1975z"/><path fill="none" stroke="#F33" stroke-width="50" d="M3000 27100H2000v-800h2000v800H3000z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="700"><tspan class="TextPosition" x="2613" y="26848"><tspan fill="#C9211E">Foo</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M4900 17289h5901v2312H4900z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="370" font-weight="400"><tspan class="TextPosition" x="7236" y="17748"><tspan fill="gray">Write operations like </tspan></tspan><tspan class="TextPosition" x="5323" y="18159"><tspan fill="gray">indexing/updating are forwarded </tspan></tspan><tspan class="TextPosition" x="8024" y="18570"><tspan fill="gray">to all instances.</tspan></tspan></tspan><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="370" font-weight="400"><tspan class="TextPosition" x="5501" y="18981"><tspan fill="gray">Read operations are forwarded </tspan></tspan><tspan class="TextPosition" x="7126" y="19392"><tspan fill="gray">to specified instance.</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M10785 15769h1422v2691h-1422z"/><path fill="none" stroke="#999" stroke-width="30" d="M10800 18444c1429 0 934-1618 1119-2337"/><path fill="#999" d="M12206 15769l-460 293 267 217 193-510z"/></g><g class="com.sun.star.drawing.ConnectorShape"><path class="BoundingBox" fill="none" d="M10785 18429h1528v2862h-1528z"/><path fill="none" stroke="#999" stroke-width="30" d="M10800 18444c1509 0 970 1782 1200 2526"/><path fill="#999" d="M12312 21290l-227-496-252 235 479 261z"/></g><g class="com.sun.star.drawing.TextShape"><path class="BoundingBox" fill="none" d="M1800 24000h7101v807H1800z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="494" font-weight="700"><tspan class="TextPosition" x="2050" y="24574"><tspan>Legend</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M13975 4275h5085v851h-5085z"/><path fill="none" stroke="#999" stroke-width="50" d="M16517 5100h-2517v-800h5034v800h-2517z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="14737" y="4848"><tspan fill="gray">ClassMethodProxy</tspan></tspan></tspan></text></g><g class="com.sun.star.drawing.CustomShape"><path class="BoundingBox" fill="none" d="M13975 5975h5085v851h-5085z"/><path fill="none" stroke="#999" stroke-width="50" d="M16517 6800h-2517v-800h5034v800h-2517z"/><text class="TextShape"><tspan class="TextParagraph" font-family="Arial, sans-serif" font-size="423" font-weight="400"><tspan class="TextPosition" x="14462" y="6548"><tspan fill="gray">InstanceMethodProxy</tspan></tspan></tspan></text></g></g></g></g></svg>
\ No newline at end of file diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md index 5f95cf3707c..777d372ec60 100644 --- a/doc/development/instrumentation.md +++ b/doc/development/instrumentation.md @@ -1,6 +1,6 @@ # Instrumenting Ruby Code -GitLab Performance Monitoring allows instrumenting of both methods and custom +[GitLab Performance Monitoring](../administration/monitoring/performance/index.md) allows instrumenting of both methods and custom blocks of Ruby code. Method instrumentation is the primary form of instrumentation with block-based instrumentation only being used when we want to drill down to specific regions of code within a method. diff --git a/doc/development/interacting_components.md b/doc/development/interacting_components.md index 74d52d808e2..5e6dc8d460a 100644 --- a/doc/development/interacting_components.md +++ b/doc/development/interacting_components.md @@ -9,8 +9,8 @@ when making _backend_ changes that might involve multiple features or [component ## Uploads -GitLab supports uploads to [object storage]. That means every feature and -change that affects uploads should also be tested against [object storage], +GitLab supports uploads to [object storage]. That means every feature and +change that affects uploads should also be tested against [object storage], which is _not_ enabled by default in [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit). When working on a related feature, make sure to enable and test it diff --git a/doc/development/kubernetes.md b/doc/development/kubernetes.md index 4b2d48903ac..f4528667814 100644 --- a/doc/development/kubernetes.md +++ b/doc/development/kubernetes.md @@ -107,7 +107,7 @@ Mitigation strategies include: ## Debugging Logs related to the Kubernetes integration can be found in -[kubernetes.log](../administration/logs.md#kuberneteslog). On a local +[`kubernetes.log`](../administration/logs.md#kuberneteslog). On a local GDK install, this will be present in `log/kubernetes.log`. Some services such as diff --git a/doc/development/lfs.md b/doc/development/lfs.md index 8c3408eb6e2..cb4c2d8967b 100644 --- a/doc/development/lfs.md +++ b/doc/development/lfs.md @@ -8,4 +8,4 @@ In April 2019, Francisco Javier López hosted a [Deep Dive] on GitLab's [Git LFS [Git LFS]: ../workflow/lfs/manage_large_binaries_with_git_lfs.html [recording on YouTube]: https://www.youtube.com/watch?v=Yyxwcksr0Qc [Google Slides]: https://docs.google.com/presentation/d/1E-aw6-z0rYd0346YhIWE7E9A65zISL9iIMAOq2zaw9E/edit -[PDF]: https://gitlab.com/gitlab-org/create-stage/uploads/07a89257a140db067bdfb484aecd35e1/Git_LFS_Deep_Dive__Create_.pdf
\ No newline at end of file +[PDF]: https://gitlab.com/gitlab-org/create-stage/uploads/07a89257a140db067bdfb484aecd35e1/Git_LFS_Deep_Dive__Create_.pdf diff --git a/doc/development/logging.md b/doc/development/logging.md index 4f63c84fc0e..b43f1029cc6 100644 --- a/doc/development/logging.md +++ b/doc/development/logging.md @@ -133,7 +133,7 @@ importer progresses. Here's what to do: logs in `/var/log/gitlab/gitlab-rails/*.log` every hour and [keep at most 30 compressed files](https://docs.gitlab.com/omnibus/settings/logs.html#logrotate). On GitLab.com, that setting is only 6 compressed files. These settings should suffice - for most users, but you may need to tweak them in [omnibus-gitlab](https://gitlab.com/gitlab-org/omnibus-gitlab). + for most users, but you may need to tweak them in [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab). 1. If you add a new file, submit an issue to the [production tracker](https://gitlab.com/gitlab-com/gl-infra/production/issues) or diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 3181b3a88cc..4740cf4de7b 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -1,12 +1,12 @@ # Migration Style Guide When writing migrations for GitLab, you have to take into account that -these will be ran by hundreds of thousands of organizations of all sizes, some with +these will be run by hundreds of thousands of organizations of all sizes, some with many years of data in their database. In addition, having to take a server offline for an upgrade small or big is a -big burden for most organizations. For this reason it is important that your -migrations are written carefully, can be applied online and adhere to the style +big burden for most organizations. For this reason, it is important that your +migrations are written carefully, can be applied online, and adhere to the style guide below. Migrations are **not** allowed to require GitLab installations to be taken @@ -85,7 +85,38 @@ be possible to downgrade in case of a vulnerability or bugs. In your migration, add a comment describing how the reversibility of the migration was tested. -## Multi Threading +## Atomicity + +By default, migrations are single transaction. That is, a transaction is opened +at the beginning of the migration, and committed after all steps are processed. + +Running migrations in a single transaction makes sure that if one of the steps fails, +none of the steps will be executed, leaving the database in valid state. +Therefore, either: + +- Put all migrations in one single-transaction migration. +- If necessary, put most actions in one migration and create a separate migration + for the steps that cannot be done in a single transaction. + +For example, if you create an empty table and need to build an index for it, +it is recommended to use a regular single-transaction migration and the default +rails schema statement: [`add_index`](https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_index). +This is a blocking operation, but it won't cause problems because the table is not yet used, +and therefore it does not have any records yet. + +## Heavy operations in a single transaction + +When using a single-transaction migration, a transaction will hold on a database connection +for the duration of the migration, so you must make sure the actions in the migration +do not take too much time: In general, queries executed in a migration need to fit comfortably +within `15s` on GitLab.com. + +In case you need to insert, update, or delete a significant amount of data, you: + +- Must disable the single transaction with `disable_ddl_transaction!`. +- Should consider doing it in a [Background Migration](background_migrations.md). + +## Multi-Threading Sometimes a migration might need to use multiple Ruby threads to speed up a migration. For this to work your migration needs to include the module @@ -122,16 +153,16 @@ pool. This ensures each thread has its own connection object, and won't time out when trying to obtain one. **NOTE:** PostgreSQL has a maximum amount of connections that it allows. This -limit can vary from installation to installation. As a result it's recommended -you do not use more than 32 threads in a single migration. Usually 4-8 threads +limit can vary from installation to installation. As a result, it's recommended +you do not use more than 32 threads in a single migration. Usually, 4-8 threads should be more than enough. ## Removing indexes -When removing an index make sure to use the method `remove_concurrent_index` instead -of the regular `remove_index` method. The `remove_concurrent_index` method -automatically drops concurrent indexes when using PostgreSQL, removing the -need for downtime. To use this method you must disable single-transaction mode +If the table is not empty when removing an index, make sure to use the method +`remove_concurrent_index` instead of the regular `remove_index` method. +The `remove_concurrent_index` method drops indexes concurrently, so no locking is required, +and there is no need for downtime. To use this method, you must disable single-transaction mode by calling the method `disable_ddl_transaction!` in the body of your migration class like so: @@ -149,19 +180,25 @@ end Note that it is not necessary to check if the index exists prior to removing it. +For a small table (such as an empty one or one with less than `1,000` records), +it is recommended to use `remove_index` in a single-transaction migration, +combining it with other operations that don't require `disable_ddl_transaction!`. + ## Adding indexes -If you need to add a unique index please keep in mind there is the possibility +If you need to add a unique index, please keep in mind there is the possibility of existing duplicates being present in the database. This means that should always _first_ add a migration that removes any duplicates, before adding the unique index. -When adding an index make sure to use the method `add_concurrent_index` instead -of the regular `add_index` method. The `add_concurrent_index` method -automatically creates concurrent indexes when using PostgreSQL, removing the -need for downtime. To use this method you must disable transactions by calling -the method `disable_ddl_transaction!` in the body of your migration class like -so: +When adding an index to a non-empty table make sure to use the method +`add_concurrent_index` instead of the regular `add_index` method. +The `add_concurrent_index` method automatically creates concurrent indexes +when using PostgreSQL, removing the need for downtime. + +To use this method, you must disable single-transactions mode +by calling the method `disable_ddl_transaction!` in the body of your migration +class like so: ```ruby class MyMigration < ActiveRecord::Migration[4.2] @@ -179,16 +216,20 @@ class MyMigration < ActiveRecord::Migration[4.2] end ``` +For a small table (such as an empty one or one with less than `1,000` records), +it is recommended to use `add_index` in a single-transaction migration, combining it with other +operations that don't require `disable_ddl_transaction!`. + ## Adding foreign-key constraints -When adding a foreign-key constraint to either an existing or new -column remember to also add a index on the column. +When adding a foreign-key constraint to either an existing or a new column also +remember to add an index on the column. This is **required** for all foreign-keys, e.g., to support efficient cascading deleting: when a lot of rows in a table get deleted, the referenced records need to be deleted too. The database has to look for corresponding records in the referenced table. Without an index, this will result in a sequential scan on the -table which can take a long time. +table, which can take a long time. Here's an example where we add a new column with a foreign key constraint. Note it includes `index: true` to create an index for it. @@ -202,13 +243,17 @@ class Migration < ActiveRecord::Migration[4.2] end ``` -When adding a foreign-key constraint to an existing column, we -have to employ `add_concurrent_foreign_key` and `add_concurrent_index` +When adding a foreign-key constraint to an existing column in a non-empty table, +we have to employ `add_concurrent_foreign_key` and `add_concurrent_index` instead of `add_reference`. +For an empty table (such as a fresh one), it is recommended to use +`add_reference` in a single-transaction migration, combining it with other +operations that don't require `disable_ddl_transaction!`. + ## Adding Columns With Default Values -When adding columns with default values you must use the method +When adding columns with default values to non-empty tables, you must use `add_column_with_default`. This method ensures the table is updated without requiring downtime. This method is not reversible so you must manually define the `up` and `down` methods in your migration class. @@ -232,10 +277,14 @@ end ``` Keep in mind that this operation can easily take 10-15 minutes to complete on -larger installations (e.g. GitLab.com). As a result you should only add default -values if absolutely necessary. There is a RuboCop cop that will fail if this -method is used on some tables that are very large on GitLab.com, which would -cause other issues. +larger installations (e.g. GitLab.com). As a result, you should only add +default values if absolutely necessary. There is a RuboCop cop that will fail if +this method is used on some tables that are very large on GitLab.com, which +would cause other issues. + +For a small table (such as an empty one or one with less than `1,000` records), +use `add_column` and `change_column_default` in a single-transaction migration, +combining it with other operations that don't require `disable_ddl_transaction!`. ## Updating an existing column @@ -253,8 +302,10 @@ update_column_in_batches(:projects, :foo, 10) do |table, query| end ``` -To perform a computed update, the value can be wrapped in `Arel.sql`, so Arel -treats it as an SQL literal. The below example is the same as the one above, but +If a computed update is needed, the value can be wrapped in `Arel.sql`, so Arel +treats it as an SQL literal. It's also a required deprecation for [Rails 6](https://gitlab.com/gitlab-org/gitlab-ce/issues/61451). + +The below example is the same as the one above, but the value is set to the product of the `bar` and `baz` columns: ```ruby @@ -275,12 +326,12 @@ staging environment - or asking someone else to do so for you - beforehand. By default, an integer column can hold up to a 4-byte (32-bit) number. That is a max value of 2,147,483,647. Be aware of this when creating a column that will -hold file sizes in byte units. If you are tracking file size in bytes this +hold file sizes in byte units. If you are tracking file size in bytes, this restricts the maximum file size to just over 2GB. To allow an integer column to hold up to an 8-byte (64-bit) number, explicitly set the limit to 8-bytes. This will allow the column to hold a value up to -9,223,372,036,854,775,807. +`9,223,372,036,854,775,807`. Rails migration example: @@ -294,9 +345,11 @@ add_column(:projects, :foo, :integer, default: 10, limit: 8) ## Timestamp column type -By default, Rails uses the `timestamp` data type that stores timestamp data without timezone information. -The `timestamp` data type is used by calling either the `add_timestamps` or the `timestamps` method. -Also Rails converts the `:datetime` data type to the `timestamp` one. +By default, Rails uses the `timestamp` data type that stores timestamp data +without timezone information. The `timestamp` data type is used by calling +either the `add_timestamps` or the `timestamps` method. + +Also, Rails converts the `:datetime` data type to the `timestamp` one. Example: @@ -317,14 +370,16 @@ def up end ``` -Instead of using these methods one should use the following methods to store timestamps with timezones: +Instead of using these methods, one should use the following methods to store +timestamps with timezones: - `add_timestamps_with_timezone` - `timestamps_with_timezone` -This ensures all timestamps have a time zone specified. This in turn means existing timestamps won't -suddenly use a different timezone when the system's timezone changes. It also makes it very clear which -timezone was used in the first place. +This ensures all timestamps have a time zone specified. This, in turn, means +existing timestamps won't suddenly use a different timezone when the system's +timezone changes. It also makes it very clear which timezone was used in the +first place. ## Storing JSON in database @@ -359,7 +414,7 @@ Make sure your migration can be reversed. ## Data migration Please prefer Arel and plain SQL over usual ActiveRecord syntax. In case of -using plain SQL you need to quote all input manually with `quote_string` helper. +using plain SQL, you need to quote all input manually with `quote_string` helper. Example with Arel: @@ -384,7 +439,7 @@ select_all("SELECT name, COUNT(id) as cnt FROM tags GROUP BY name HAVING COUNT(i end ``` -If you need more complex logic you can define and use models local to a +If you need more complex logic, you can define and use models local to a migration. For example: ```ruby @@ -395,13 +450,13 @@ class MyMigration < ActiveRecord::Migration[4.2] end ``` -When doing so be sure to explicitly set the model's table name so it's not +When doing so be sure to explicitly set the model's table name, so it's not derived from the class name or namespace. ### Renaming reserved paths -When a new route for projects is introduced that could conflict with any -existing records. The path for this records should be renamed, and the +When a new route for projects is introduced, it could conflict with any +existing records. The path for these records should be renamed, and the related data should be moved on disk. Since we had to do this a few times already, there are now some helpers to help diff --git a/doc/development/namespaces_storage_statistics.md b/doc/development/namespaces_storage_statistics.md new file mode 100644 index 00000000000..2c7e5935435 --- /dev/null +++ b/doc/development/namespaces_storage_statistics.md @@ -0,0 +1,178 @@ +# Database case study: Namespaces storage statistics + +## Introduction + +On [Storage and limits management for groups](https://gitlab.com/groups/gitlab-org/-/epics/886), +we want to facilitate a method for easily viewing the amount of +storage consumed by a group, and allow easy management. + +## Proposal + +1. Create a new ActiveRecord model to hold the namespaces' statistics in an aggregated form (only for root namespaces). +1. Refresh the statistics in this model every time a project belonging to this namespace is changed. + +## Problem + +In GitLab, we update the project storage statistics through a +[callback](https://gitlab.com/gitlab-org/gitlab-ce/blob/v12.2.0.pre/app/models/project.rb#L90) +every time the project is saved. + +The summary of those statistics per namespace is then retrieved +by [`Namespaces#with_statistics`](https://gitlab.com/gitlab-org/gitlab-ce/blob/v12.2.0.pre/app/models/namespace.rb#L70) scope. Analyzing this query we noticed that: + +- It takes up to `1.2` seconds for namespaces with over `15k` projects. +- It can't be analyzed with [ChatOps](chatops_on_gitlabcom.md), as it times out. + +Additionally, the pattern that is currently used to update the project statistics +(the callback) doesn't scale adequately. It is currently one of the largest +[database queries transactions on production](https://gitlab.com/gitlab-org/gitlab-ce/issues/62488) +that takes the most time overall. We can't add one more query to it as +it will increase the transaction's length. + +Because of all of the above, we can't apply the same pattern to store +and update the namespaces statistics, as the `namespaces` table is one +of the largest tables on GitLab.com. Therefore we needed to find a performant and +alternative method. + +## Attempts + +### Attempt A: PostgreSQL materialized view + +Model can be updated through a refresh strategy based on a project routes SQL and a [materialized view](https://www.postgresql.org/docs/9.6/rules-materializedviews.html): + +```sql +SELECT split_part("rs".path, '/', 1) as root_path, + COALESCE(SUM(ps.storage_size), 0) AS storage_size, + COALESCE(SUM(ps.repository_size), 0) AS repository_size, + COALESCE(SUM(ps.wiki_size), 0) AS wiki_size, + COALESCE(SUM(ps.lfs_objects_size), 0) AS lfs_objects_size, + COALESCE(SUM(ps.build_artifacts_size), 0) AS build_artifacts_size, + COALESCE(SUM(ps.packages_size), 0) AS packages_size +FROM "projects" + INNER JOIN routes rs ON rs.source_id = projects.id AND rs.source_type = 'Project' + INNER JOIN project_statistics ps ON ps.project_id = projects.id +GROUP BY root_path +``` + +We could then execute the query with: + +```sql +REFRESH MATERIALIZED VIEW root_namespace_storage_statistics; +``` + +While this implied a single query update (and probably a fast one), it has some downsides: + +- Materialized views syntax varies from PostgreSQL and MySQL. While this feature was worked on, MySQL was still supported by GitLab. +- Rails does not have native support for materialized views. We'd need to use a specialized gem to take care of the management of the database views, which implies additional work. + +### Attempt B: An update through a CTE + +Similar to Attempt A: Model update done through a refresh strategy with a [Common Table Expression](https://www.postgresql.org/docs/9.1/queries-with.html) + +```sql +WITH refresh AS ( + SELECT split_part("rs".path, '/', 1) as root_path, + COALESCE(SUM(ps.storage_size), 0) AS storage_size, + COALESCE(SUM(ps.repository_size), 0) AS repository_size, + COALESCE(SUM(ps.wiki_size), 0) AS wiki_size, + COALESCE(SUM(ps.lfs_objects_size), 0) AS lfs_objects_size, + COALESCE(SUM(ps.build_artifacts_size), 0) AS build_artifacts_size, + COALESCE(SUM(ps.packages_size), 0) AS packages_size + FROM "projects" + INNER JOIN routes rs ON rs.source_id = projects.id AND rs.source_type = 'Project' + INNER JOIN project_statistics ps ON ps.project_id = projects.id + GROUP BY root_path) +UPDATE namespace_storage_statistics +SET storage_size = refresh.storage_size, + repository_size = refresh.repository_size, + wiki_size = refresh.wiki_size, + lfs_objects_size = refresh.lfs_objects_size, + build_artifacts_size = refresh.build_artifacts_size, + packages_size = refresh.packages_size +FROM refresh + INNER JOIN routes rs ON rs.path = refresh.root_path AND rs.source_type = 'Namespace' +WHERE namespace_storage_statistics.namespace_id = rs.source_id +``` + +Same benefits and downsides as attempt A. + +### Attempt C: Get rid of the model and store the statistics on Redis + +We could get rid of the model that stores the statistics in aggregated form and instead use a Redis Set. +This would be the [boring solution](https://about.gitlab.com/handbook/values/#boring-solutions) and the fastest one +to implement, as GitLab already includes Redis as part of its [Architecture](architecture.md#redis). + +The downside of this approach is that Redis does not provide the same persistence/consistency guarantees as PostgreSQL, +and this is information we can't afford to lose in a Redis failure. + +### Attempt D: Tag the root namespace and its child namespaces + +Directly relate the root namespace to its child namespaces, so +whenever a namespace is created without a parent, this one is tagged +with the root namespace ID: + +| id | root_id | parent_id +|:---|:--------|:---------- +| 1 | 1 | NULL +| 2 | 1 | 1 +| 3 | 1 | 2 + +To aggregate the statistics inside a namespace we'd execute something like: + +```sql +SELECT COUNT(...) +FROM projects +WHERE namespace_id IN ( + SELECT id + FROM namespaces + WHERE root_id = X +) +``` + +Even though this approach would make aggregating much easier, it has some major downsides: + +- We'd have to migrate **all namespaces** by adding and filling a new column. Because of the size of the table, dealing with time/cost will not be great. The background migration will take approximately `153h`, see <https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29772>. +- Background migration has to be shipped one release before, delaying the functionality by another milestone. + +### Attempt E (final): Update the namespace storage statistics in async way + +This approach consists of keep using the incremental statistics updates we currently already have, +but we refresh them through Sidekiq jobs and in different transactions: + +1. Create a second table (`namespace_aggregation_schedules`) with two columns `id` and `namespace_id`. +1. Whenever the statistics of a project changes, insert a row into `namespace_aggregation_schedules` + - We don't insert a new row if there's already one related to the root namespace. + - Keeping in mind the length of the transaction that involves updating `project_statistics`(<https://gitlab.com/gitlab-org/gitlab-ce/issues/62488>), the insertion should be done in a different transaction and through a Sidekiq Job. +1. After inserting the row, we schedule another worker to be executed async at two different moments: + - One enqueued for immediate execution and another one scheduled in `1.5h` hours. + - We only schedule the jobs, if we can obtain a `1.5h` lease on Redis on a key based on the root namespace ID. + - If we can't obtain the lease, it indicates there's another aggregation already in progress, or scheduled in no more than `1.5h`. +1. This worker will: + - Update the root namespace storage statistics by querying all the namespaces through a service. + - Delete the related `namespace_aggregation_schedules` after the update. +1. Another Sidekiq job is also included to traverse any remaining rows on the `namespace_aggregation_schedules` table and schedule jobs for every pending row. + - This job is scheduled with cron to run every night (UTC). + +This implementation has the following benefits: + +- All the updates are done async, so we're not increasing the length of the transactions for `project_statistics`. +- We're doing the update in a single SQL query. +- It is compatible with PostgreSQL and MySQL. +- No background migration required. + +The only downside of this approach is that namespaces' statistics are updated up to `1.5` hours after the change is done, +which means there's a time window in which the statistics are inaccurate. Because we're still not +[enforcing storage limits](https://gitlab.com/gitlab-org/gitlab-ce/issues/30421), this is not a major problem. + +## Conclusion + +Updating the storage statistics asynchronously, was the less problematic and +performant approach of aggregating the root namespaces. + +All the details regarding this use case can be found on: + +- <https://gitlab.com/gitlab-org/gitlab-ce/issues/62214> +- Merge Request with the implementation: <https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28996> + +Performance of the namespace storage statistics were measured in staging and production (GitLab.com). All results were posted +on <https://gitlab.com/gitlab-org/gitlab-ce/issues/64092>: No problem has been reported so far. diff --git a/doc/development/new_fe_guide/dependencies.md b/doc/development/new_fe_guide/dependencies.md index 8a6930acd37..161ffb1fb57 100644 --- a/doc/development/new_fe_guide/dependencies.md +++ b/doc/development/new_fe_guide/dependencies.md @@ -1,6 +1,6 @@ # Dependencies -## Adding Dependencies. +## Adding Dependencies GitLab uses `yarn` to manage dependencies. These dependencies are defined in two groups within `package.json`, `dependencies` and `devDependencies`. For diff --git a/doc/development/new_fe_guide/development/testing.md b/doc/development/new_fe_guide/development/testing.md index f7ea496d935..b990425ca3c 100644 --- a/doc/development/new_fe_guide/development/testing.md +++ b/doc/development/new_fe_guide/development/testing.md @@ -1,361 +1,5 @@ -# Overview of Frontend Testing +--- +redirect_to: '../../testing_guide/frontend_testing.md' +--- -Tests relevant for frontend development can be found at the following places: - -- `spec/javascripts/` which are run by Karma (command: `yarn karma`) and contain - - [frontend unit tests](#frontend-unit-tests) - - [frontend component tests](#frontend-component-tests) - - [frontend integration tests](#frontend-integration-tests) -- `spec/frontend/` which are run by Jest (command: `yarn jest`) and contain - - [frontend unit tests](#frontend-unit-tests) - - [frontend component tests](#frontend-component-tests) - - [frontend integration tests](#frontend-integration-tests) -- `spec/features/` which are run by RSpec and contain - - [feature tests](#feature-tests) - -All tests in `spec/javascripts/` will eventually be migrated to `spec/frontend/` (see also [#52483](https://gitlab.com/gitlab-org/gitlab-ce/issues/52483)). - -In addition there were feature tests in `features/` run by Spinach in the past. -These have been removed from our codebase in May 2018 ([#23036](https://gitlab.com/gitlab-org/gitlab-ce/issues/23036)). - -See also: - -- [Old testing guide](../../testing_guide/frontend_testing.html). -- [Notes on testing Vue components](../../fe_guide/vue.html#testing-vue-components). - -## Frontend unit tests - -Unit tests are on the lowest abstraction level and typically test functionality that is not directly perceivable by a user. - -### When to use unit tests - -<details> - <summary>exported functions and classes</summary> - Anything that is exported can be reused at various places in a way you have no control over. - Therefore it is necessary to document the expected behavior of the public interface with tests. -</details> - -<details> - <summary>Vuex actions</summary> - Any Vuex action needs to work in a consistent way independent of the component it is triggered from. -</details> - -<details> - <summary>Vuex mutations</summary> - For complex Vuex mutations it helps to identify the source of a problem by separating the tests from other parts of the Vuex store. -</details> - -### When *not* to use unit tests - -<details> - <summary>non-exported functions or classes</summary> - Anything that is not exported from a module can be considered private or an implementation detail and doesn't need to be tested. -</details> - -<details> - <summary>constants</summary> - Testing the value of a constant would mean to copy it. - This results in extra effort without additional confidence that the value is correct. -</details> - -<details> - <summary>Vue components</summary> - Computed properties, methods, and lifecycle hooks can be considered an implementation detail of components and don't need to be tested. - They are implicitly covered by component tests. - The <a href="https://vue-test-utils.vuejs.org/guides/#getting-started">official Vue guidelines</a> suggest the same. -</details> - -### What to mock in unit tests - -<details> - <summary>state of the class under test</summary> - Modifying the state of the class under test directly rather than using methods of the class avoids side-effects in test setup. -</details> - -<details> - <summary>other exported classes</summary> - Every class needs to be tested in isolation to prevent test scenarios from growing exponentially. -</details> - -<details> - <summary>single DOM elements if passed as parameters</summary> - For tests that only operate on single DOM elements rather than a whole page, creating these elements is cheaper than loading a whole HTML fixture. -</details> - -<details> - <summary>all server requests</summary> - When running frontend unit tests, the backend may not be reachable. - Therefore all outgoing requests need to be mocked. -</details> - -<details> - <summary>asynchronous background operations</summary> - Background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. -</details> - -### What *not* to mock in unit tests - -<details> - <summary>non-exported functions or classes</summary> - Everything that is not exported can be considered private to the module and will be implicitly tested via the exported classes / functions. -</details> - -<details> - <summary>methods of the class under test</summary> - By mocking methods of the class under test, the mocks will be tested and not the real methods. -</details> - -<details> - <summary>utility functions (pure functions, or those that only modify parameters)</summary> - If a function has no side effects because it has no state, it is safe to not mock it in tests. -</details> - -<details> - <summary>full HTML pages</summary> - Loading the HTML of a full page slows down tests, so it should be avoided in unit tests. -</details> - -## Frontend component tests - -Component tests cover the state of a single component that is perceivable by a user depending on external signals such as user input, events fired from other components, or application state. - -### When to use component tests - -- Vue components - -### When *not* to use component tests - -<details> - <summary>Vue applications</summary> - Vue applications may contain many components. - Testing them on a component level requires too much effort. - Therefore they are tested on frontend integration level. -</details> - -<details> - <summary>HAML templates</summary> - HAML templates contain only Markup and no frontend-side logic. - Therefore they are not complete components. -</details> - -### What to mock in component tests - -<details> - <summary>DOM</summary> - Operating on the real DOM is significantly slower than on the virtual DOM. -</details> - -<details> - <summary>properties and state of the component under test</summary> - Similarly to testing classes, modifying the properties directly (rather than relying on methods of the component) avoids side-effects. -</details> - -<details> - <summary>Vuex store</summary> - To avoid side effects and keep component tests simple, Vuex stores are replaced with mocks. -</details> - -<details> - <summary>all server requests</summary> - Similar to unit tests, when running component tests, the backend may not be reachable. - Therefore all outgoing requests need to be mocked. -</details> - -<details> - <summary>asynchronous background operations</summary> - Similar to unit tests, background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. -</details> - -<details> - <summary>child components</summary> - Every component is tested individually, so child components are mocked. - See also <a href="https://vue-test-utils.vuejs.org/api/#shallowmount">shallowMount()</a> -</details> - -### What *not* to mock in component tests - -<details> - <summary>methods or computed properties of the component under test</summary> - By mocking part of the component under test, the mocks will be tested and not the real component. -</details> - -<details> - <summary>functions and classes independent from Vue</summary> - All plain JavaScript code is already covered by unit tests and needs not to be mocked in component tests. -</details> - -## Frontend integration tests - -Integration tests cover the interaction between all components on a single page. -Their abstraction level is comparable to how a user would interact with the UI. - -### When to use integration tests - -<details> - <summary>page bundles (<code>index.js</code> files in <code>app/assets/javascripts/pages/</code>)</summary> - Testing the page bundles ensures the corresponding frontend components integrate well. -</details> - -<details> - <summary>Vue applications outside of page bundles</summary> - Testing Vue applications as a whole ensures the corresponding frontend components integrate well. -</details> - -### What to mock in integration tests - -<details> - <summary>HAML views (use fixtures instead)</summary> - Rendering HAML views requires a Rails environment including a running database which we cannot rely on in frontend tests. -</details> - -<details> - <summary>all server requests</summary> - Similar to unit and component tests, when running component tests, the backend may not be reachable. - Therefore all outgoing requests need to be mocked. -</details> - -<details> - <summary>asynchronous background operations that are not perceivable on the page</summary> - Background operations that affect the page need to be tested on this level. - All other background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. -</details> - -### What *not* to mock in integration tests - -<details> - <summary>DOM</summary> - Testing on the real DOM ensures our components work in the environment they are meant for. - Part of this will be delegated to <a href="https://gitlab.com/gitlab-org/quality/team-tasks/issues/45">cross-browser testing</a>. -</details> - -<details> - <summary>properties or state of components</summary> - On this level, all tests can only perform actions a user would do. - For example to change the state of a component, a click event would be fired. -</details> - -<details> - <summary>Vuex stores</summary> - When testing the frontend code of a page as a whole, the interaction between Vue components and Vuex stores is covered as well. -</details> - -## Feature tests - -In contrast to [frontend integration tests](#frontend-integration-tests), feature tests make requests against the real backend instead of using fixtures. -This also implies that database queries are executed which makes this category significantly slower. - -See also the [RSpec testing guidelines](../../testing_guide/best_practices.md#rspec). - -### When to use feature tests - -- use cases that require a backend and cannot be tested using fixtures -- behavior that is not part of a page bundle but defined globally - -### Relevant notes - -A `:js` flag is added to the test to make sure the full environment is loaded. - -``` -scenario 'successfully', :js do - sign_in(create(:admin)) -end -``` - -The steps of each test are written using capybara methods ([documentation](https://www.rubydoc.info/gems/capybara)). - -Bear in mind <abbr title="XMLHttpRequest">XHR</abbr> calls might require you to use `wait_for_requests` in between steps, like so: - -```rspec -find('.form-control').native.send_keys(:enter) - -wait_for_requests - -expect(page).not_to have_selector('.card') -``` - -## Test helpers - -### Vuex Helper: `testAction` - -We have a helper available to make testing actions easier, as per [official documentation](https://vuex.vuejs.org/guide/testing.html): - -``` -testAction( - actions.actionName, // action - { }, // params to be passed to action - state, // state - [ - { type: types.MUTATION}, - { type: types.MUTATION_1, payload: {}}, - ], // mutations committed - [ - { type: 'actionName', payload: {}}, - { type: 'actionName1', payload: {}}, - ] // actions dispatched - done, -); -``` - -Check an example in [spec/javascripts/ide/stores/actions_spec.jsspec/javascripts/ide/stores/actions_spec.js](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/ide/stores/actions_spec.js). - -### Vue Helper: `mountComponent` - -To make mounting a Vue component easier and more readable, we have a few helpers available in `spec/helpers/vue_mount_component_helper`. - -- `createComponentWithStore` -- `mountComponentWithStore` - -Examples of usage: - -``` -beforeEach(() => { - vm = createComponentWithStore(Component, store); - - vm.$store.state.currentBranchId = 'master'; - - vm.$mount(); -}, -``` - -``` -beforeEach(() => { - vm = mountComponentWithStore(Component, { - el: '#dummy-element', - store, - props: { badge }, - }); -}, -``` - -Don't forget to clean up: - -``` -afterEach(() => { - vm.$destroy(); -}); -``` - -## Testing with older browsers - -Some regressions only affect a specific browser version. We can install and test in particular browsers with either Firefox or Browserstack using the following steps: - -### Browserstack - -[Browserstack](https://www.browserstack.com/) allows you to test more than 1200 mobile devices and browsers. -You can use it directly through the [live app](https://www.browserstack.com/live) or you can install the [chrome extension](https://chrome.google.com/webstore/detail/browserstack/nkihdmlheodkdfojglpcjjmioefjahjb) for easy access. -You can find the credentials on 1Password, under `frontendteam@gitlab.com`. - -### Firefox - -#### macOS - -You can download any older version of Firefox from the releases FTP server, <https://ftp.mozilla.org/pub/firefox/releases/> - -1. From the website, select a version, in this case `50.0.1`. -1. Go to the mac folder. -1. Select your preferred language, you will find the dmg package inside, download it. -1. Drag and drop the application to any other folder but the `Applications` folder. -1. Rename the application to something like `Firefox_Old`. -1. Move the application to the `Applications` folder. -1. Open up a terminal and run `/Applications/Firefox_Old.app/Contents/MacOS/firefox-bin -profilemanager` to create a new profile specific to that Firefox version. -1. Once the profile has been created, quit the app, and run it again like normal. You now have a working older Firefox version. +This document was moved to [another location](../../testing_guide/frontend_testing.md). diff --git a/doc/development/new_fe_guide/modules/dirty_submit.md b/doc/development/new_fe_guide/modules/dirty_submit.md index 6c03958b463..217743ea395 100644 --- a/doc/development/new_fe_guide/modules/dirty_submit.md +++ b/doc/development/new_fe_guide/modules/dirty_submit.md @@ -1,7 +1,6 @@ # Dirty Submit -> [Introduced][ce-21115] in GitLab 11.3. -> [dirty_submit][dirty-submit] +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21115) in GitLab 11.3. ## Summary @@ -9,6 +8,9 @@ Prevent submitting forms with no changes. Currently handles `input`, `textarea` and `select` elements. +Also, see [the code](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/dirty_submit/) +within the GitLab project. + ## Usage ```js @@ -18,6 +20,3 @@ new DirtySubmitForm(document.querySelector('form')); // or new DirtySubmitForm(document.querySelectorAll('form')); ``` - -[ce-21115]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21115 -[dirty-submit]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/dirty_submit/
\ No newline at end of file diff --git a/doc/development/new_fe_guide/style/javascript.md b/doc/development/new_fe_guide/style/javascript.md index 802ebd12d92..b742d567f41 100644 --- a/doc/development/new_fe_guide/style/javascript.md +++ b/doc/development/new_fe_guide/style/javascript.md @@ -192,4 +192,4 @@ rules only if you are invoking/instantiating existing code modules. - [class-method-use-this](http://eslint.org/docs/rules/class-methods-use-this) > Note: Disable these rules on a per line basis. This makes it easier to refactor - in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`. +> in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`. diff --git a/doc/development/omnibus.md b/doc/development/omnibus.md index 0ba354d28a2..ea5c18f1a8c 100644 --- a/doc/development/omnibus.md +++ b/doc/development/omnibus.md @@ -1,32 +1,32 @@ -# What you should know about omnibus packages +# What you should know about Omnibus packages -Most users install GitLab using our omnibus packages. As a developer it can be -good to know how the omnibus packages differ from what you have on your laptop +Most users install GitLab using our Omnibus packages. As a developer it can be +good to know how the Omnibus packages differ from what you have on your laptop when you are coding. ## Files are owned by root by default -All the files in the Rails tree (`app/`, `config/` etc.) are owned by 'root' in -omnibus installations. This makes the installation simpler and it provides -extra security. The omnibus reconfigure script contains commands that give -write access to the 'git' user only where needed. +All the files in the Rails tree (`app/`, `config/` etc.) are owned by `root` in +Omnibus installations. This makes the installation simpler and it provides +extra security. The Omnibus reconfigure script contains commands that give +write access to the `git` user only where needed. -For example, the 'git' user is allowed to write in the `log/` directory, in +For example, the `git` user is allowed to write in the `log/` directory, in `public/uploads`, and they are allowed to rewrite the `db/schema.rb` file. In other cases, the reconfigure script tricks GitLab into not trying to write a file. For instance, GitLab will generate a `.secret` file if it cannot find one -and write it to the Rails root. In the omnibus packages, reconfigure writes the +and write it to the Rails root. In the Omnibus packages, reconfigure writes the `.secret` file first, so that GitLab never tries to write it. ## Code, data and logs are in separate directories -The omnibus design separates code (read-only, under `/opt/gitlab`) from data +The Omnibus design separates code (read-only, under `/opt/gitlab`) from data (read/write, under `/var/opt/gitlab`) and logs (read/write, under `/var/log/gitlab`). To make this happen the reconfigure script sets custom paths where it can in GitLab config files, and where there are no path settings, it uses symlinks. For example, `config/gitlab.yml` is treated as data so that file is a symlink. -The same goes for `public/uploads`. The `log/` directory is replaced by omnibus +The same goes for `public/uploads`. The `log/` directory is replaced by Omnibus with a symlink to `/var/log/gitlab/gitlab-rails`. diff --git a/doc/development/python_guide/index.md b/doc/development/python_guide/index.md index a80bee27d4a..47d9d96766c 100644 --- a/doc/development/python_guide/index.md +++ b/doc/development/python_guide/index.md @@ -7,9 +7,9 @@ As of GitLab 11.10, we require Python 3. ## Installation -There are several ways of installing python on your system. To be able to use the same version we use in production, -we suggest you use [pyenv](https://github.com/pyenv/pyenv). It works and behave similar to its counterpart in the -ruby world: [rbenv](https://github.com/rbenv/rbenv). +There are several ways of installing Python on your system. To be able to use the same version we use in production, +we suggest you use [pyenv](https://github.com/pyenv/pyenv). It works and behaves similarly to its counterpart in the +Ruby world: [rbenv](https://github.com/rbenv/rbenv). ### macOS @@ -67,7 +67,7 @@ Running this command will install both the required Python version as well as re ## Use instructions -To run any python code under the Pipenv environment, you need to first start a `virtualenv` based on the dependencies +To run any Python code under the Pipenv environment, you need to first start a `virtualenv` based on the dependencies of the application. With Pipenv, this is a simple as running: ```bash diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index a6b60149ea4..3787e2ef187 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -36,6 +36,13 @@ it "avoids N+1 database queries" do end ``` +## Use request specs instead of controller specs + +Use a [request spec](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/spec/requests) when writing a N+1 test on the controller level. + +Controller specs should not be used to write N+1 tests as the controller is only initialized once per example. +This could lead to false successes where subsequent "requests" could have queries reduced (e.g. because of memoization). + ## Finding the source of the query It may be useful to identify the source of the queries by looking at the call backtrace. diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index 67f36eb1ab4..e9d6cfe00b2 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -9,7 +9,7 @@ bundle exec rake setup ``` The `setup` task is an alias for `gitlab:setup`. -This tasks calls `db:reset` to create the database, calls `add_limits_mysql` that adds limits to the database schema in case of a MySQL database and finally it calls `db:seed_fu` to seed the database. +This tasks calls `db:reset` to create the database, and calls `db:seed_fu` to seed the database. Note: `db:setup` calls `db:seed` but this does nothing. ### Seeding issues for all or a given project @@ -216,4 +216,3 @@ bundle exec rake routes Since these take some time to create, it's often helpful to save the output to a file for quick reference. - diff --git a/doc/development/repository_mirroring.md b/doc/development/repository_mirroring.md index f8c33ff2b85..dc51bf80e92 100644 --- a/doc/development/repository_mirroring.md +++ b/doc/development/repository_mirroring.md @@ -8,4 +8,4 @@ In December 2018, Tiago Botelho hosted a [Deep Dive] on GitLab's [Pull Repositor [Pull Repository Mirroring functionality]: ../workflow/repository_mirroring.md#pulling-from-a-remote-repository-starter [recording on YouTube]: https://www.youtube.com/watch?v=sSZq0fpdY-Y [Google Slides]: https://docs.google.com/presentation/d/17BTT6M6RyNRckV4wTt-dr07nIfBvD325_xVBoLtSoPM/edit?usp=sharing -[PDF]: https://gitlab.com/gitlab-org/create-stage/uploads/8693404888a941fd851f8a8ecdec9675/Gitlab_Create_-_Pull_Mirroring_Deep_Dive.pdf
\ No newline at end of file +[PDF]: https://gitlab.com/gitlab-org/create-stage/uploads/8693404888a941fd851f8a8ecdec9675/Gitlab_Create_-_Pull_Mirroring_Deep_Dive.pdf diff --git a/doc/development/session.md b/doc/development/session.md index 9edce3dbda0..971795d8816 100644 --- a/doc/development/session.md +++ b/doc/development/session.md @@ -17,7 +17,7 @@ When storing values in a session it is best to: - Use simple primitives and avoid storing objects to avoid marshaling complications. - Clean up after unneeded variables to keep memory usage in Redis down. -## Gitlab::Session +## GitLab::Session Sometimes you might want to persist data in the session instead of another store like the database. `Gitlab::Session` lets you access this without passing the session around extensively. For example, you could access it from within a policy without having to pass the session through to each place permissions are checked from. diff --git a/doc/development/sha1_as_binary.md b/doc/development/sha1_as_binary.md index 3151cc29bbc..6c4252ec634 100644 --- a/doc/development/sha1_as_binary.md +++ b/doc/development/sha1_as_binary.md @@ -2,7 +2,7 @@ Storing SHA1 hashes as strings is not very space efficient. A SHA1 as a string requires at least 40 bytes, an additional byte to store the encoding, and -perhaps more space depending on the internals of PostgreSQL and MySQL. +perhaps more space depending on the internals of PostgreSQL. On the other hand, if one were to store a SHA1 as binary one would only need 20 bytes for the actual SHA1, and 1 or 4 bytes of additional space (again depending diff --git a/doc/development/shell_commands.md b/doc/development/shell_commands.md index 7bdf676be58..1300c99622e 100644 --- a/doc/development/shell_commands.md +++ b/doc/development/shell_commands.md @@ -35,7 +35,7 @@ Gitlab::Popen.popen(%W(find /some/path -not -path /some/path -mmin +120 -delete) This coding style could have prevented CVE-2013-4490. -## Always use the configurable git binary path for git commands +## Always use the configurable Git binary path for Git commands ```ruby # Wrong @@ -114,7 +114,7 @@ user = `whoami` user, exit_status = Gitlab::Popen.popen(%W(whoami)) ``` -In other repositories, such as gitlab-shell you can also use `IO.popen`. +In other repositories, such as GitLab Shell you can also use `IO.popen`. ```ruby # Safe IO.popen example diff --git a/doc/development/shell_scripting_guide/index.md b/doc/development/shell_scripting_guide/index.md index ae7f2154682..0809f8b1a0a 100644 --- a/doc/development/shell_scripting_guide/index.md +++ b/doc/development/shell_scripting_guide/index.md @@ -24,7 +24,8 @@ Having said all of the above, we recommend staying away from shell scripts as much as possible. A language like Ruby or Python (if required for consistency with codebases that we leverage) is almost always a better choice. The high-level interpreted languages have more readable syntax, offer much more -mature capabilities for unit-testing, linting, and error reporting. +mature capabilities for unit-testing, linting, and error reporting. + Use shell scripts only if there's a strong restriction on project's dependencies size or any other requirements that are more important in a particular case. @@ -48,12 +49,12 @@ that is: This section describes the tools that should be made a mandatory part of a project's CI pipeline if it contains shell scripts. These tools -automate shell code formatting, checking for errors or vulnerabilities, etc. +automate shell code formatting, checking for errors or vulnerabilities, etc. ### Linting We're using the [ShellCheck](https://www.shellcheck.net/) utility in its default configuration to lint our -shell scripts. +shell scripts. All projects with shell scripts should use this GitLab CI/CD job: @@ -98,7 +99,7 @@ NOTE: **Note:** This is a work in progress. It is an [ongoing effort](https://gitlab.com/gitlab-org/gitlab-ce/issues/64016) to evaluate different tools for the -automated testing of shell scripts (like [BATS](https://github.com/sstephenson/bats)). +automated testing of shell scripts (like [BATS](https://github.com/sstephenson/bats)). ## Code Review diff --git a/doc/development/sql.md b/doc/development/sql.md index a256fd46c09..2584dcfb4ca 100644 --- a/doc/development/sql.md +++ b/doc/development/sql.md @@ -15,14 +15,11 @@ FROM issues WHERE title LIKE 'WIP:%'; ``` -On PostgreSQL the `LIKE` statement is case-sensitive. On MySQL this depends on -the case-sensitivity of the collation, which is usually case-insensitive. To -perform a case-insensitive `LIKE` on PostgreSQL you have to use `ILIKE` instead. -This statement in turn isn't supported on MySQL. +On PostgreSQL the `LIKE` statement is case-sensitive. To perform a case-insensitive +`LIKE` you have to use `ILIKE` instead. -To work around this problem you should write `LIKE` queries using Arel instead -of raw SQL fragments as Arel automatically uses `ILIKE` on PostgreSQL and `LIKE` -on MySQL. This means that instead of this: +To handle this automatically you should use `LIKE` queries using Arel instead +of raw SQL fragments, as Arel automatically uses `ILIKE` on PostgreSQL. ```ruby Issue.where('title LIKE ?', 'WIP:%') @@ -45,7 +42,7 @@ table = Issue.arel_table Issue.where(table[:title].matches('WIP:%').or(table[:foo].matches('WIP:%'))) ``` -For PostgreSQL this produces: +On PostgreSQL, this produces: ```sql SELECT * @@ -53,18 +50,10 @@ FROM issues WHERE (title ILIKE 'WIP:%' OR foo ILIKE 'WIP:%') ``` -In turn for MySQL this produces: - -```sql -SELECT * -FROM issues -WHERE (title LIKE 'WIP:%' OR foo LIKE 'WIP:%') -``` - ## LIKE & Indexes -Neither PostgreSQL nor MySQL use any indexes when using `LIKE` / `ILIKE` with a -wildcard at the start. For example, this will not use any indexes: +PostgreSQL won't use any indexes when using `LIKE` / `ILIKE` with a wildcard at +the start. For example, this will not use any indexes: ```sql SELECT * @@ -75,9 +64,8 @@ WHERE title ILIKE '%WIP:%'; Because the value for `ILIKE` starts with a wildcard the database is not able to use an index as it doesn't know where to start scanning the indexes. -MySQL provides no known solution to this problem. Luckily PostgreSQL _does_ -provide a solution: trigram GIN indexes. These indexes can be created as -follows: +Luckily, PostgreSQL _does_ provide a solution: trigram GIN indexes. These +indexes can be created as follows: ```sql CREATE INDEX [CONCURRENTLY] index_name_here diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 9d6792e9139..0f982c3a48b 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -13,17 +13,7 @@ a level that is difficult to manage. Test heuristics can help solve this problem. They concisely address many of the common ways bugs manifest themselves within our code. When designing our tests, take time to review known test heuristics to inform our test design. We can find some helpful heuristics documented in the Handbook in the -[Test Design](https://about.gitlab.com/handbook/engineering/quality/guidelines/test-engineering/test-design/) section. - -## Run tests against MySQL - -By default, tests are only run against PostgreSQL, but you can run them on -demand against MySQL by following one of the following conventions: - -| Convention | Valid example | -|:----------------------|:-----------------------------| -| Include `mysql` in your branch name | `enhance-mysql-support` | -| Include `[run mysql]` in your commit message | `Fix MySQL support<br><br>[run mysql]` | +[Test Engineering](https://about.gitlab.com/handbook/engineering/quality/test-engineering/#test-heuristics) section. ## Test speed @@ -455,6 +445,19 @@ complexity of RSpec expectations.They should be placed under a certain type of specs only (e.g. features, requests etc.) but shouldn't be if they apply to multiple type of specs. +#### `be_like_time` + +Time returned from a database can differ in precision from time objects +in Ruby, so we need flexible tolerances when comparing in specs. We can +use `be_like_time` to compare that times are within one second of each +other. + +Example: + +```ruby +expect(metrics.merged_at).to be_like_time(time) +``` + #### `have_gitlab_http_status` Prefer `have_gitlab_http_status` over `have_http_status` because the former diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md index 87d48726268..d9f66a827de 100644 --- a/doc/development/testing_guide/ci.md +++ b/doc/development/testing_guide/ci.md @@ -39,7 +39,6 @@ slowest test files and try to improve them. ## CI setup -- On CE and EE, the test suite runs both PostgreSQL and MySQL. - Rails logging to `log/test.log` is disabled by default in CI [for performance reasons][logging]. To override this setting, provide the `RAILS_ENABLE_TEST_LOG` environment variable. diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index d6b944a3e74..3ae3ce183d9 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -45,11 +45,11 @@ Results are reported in the `#qa-staging` Slack channel. ### Testing code in merge requests -#### Using the `package-and-qa` job +#### Using the `package-and-qa-manual` job It is possible to run end-to-end tests for a merge request, eventually being run in a pipeline in the [`gitlab-qa`](https://gitlab.com/gitlab-org/gitlab-qa/) project, -by triggering the `package-and-qa` manual action in the `test` stage (not +by triggering the `package-and-qa-manual` manual action in the `test` stage (not available for forks). **This runs end-to-end tests against a custom Omnibus package built from your @@ -71,7 +71,7 @@ graph LR B2[`Trigger-qa` stage<br>`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 subgraph "gitlab-ce/ee pipeline" - A1[`test` stage<br>`package-and-qa` job] + A1[`test` stage<br>`package-and-qa-manual` job] end subgraph "omnibus-gitlab pipeline" @@ -79,7 +79,7 @@ subgraph "omnibus-gitlab pipeline" end subgraph "gitlab-qa pipeline" - A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa` job<br>and post the result on the original commit tested| A1 + A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa-manual` job<br>and post the result on the original commit tested| A1 end ``` diff --git a/doc/development/testing_guide/end_to_end/page_objects.md b/doc/development/testing_guide/end_to_end/page_objects.md index 47e58a425fd..850ea6b60ac 100644 --- a/doc/development/testing_guide/end_to_end/page_objects.md +++ b/doc/development/testing_guide/end_to_end/page_objects.md @@ -40,7 +40,7 @@ the time it would take to build packages and test everything. That is why when someone changes `t.text_field :login` to `t.text_field :username` in the _new session_ view we won't know about this change until our GitLab QA nightly pipeline fails, or until someone triggers -`package-and-qa` action in their merge request. +`package-and-qa-manual` action in their merge request. Obviously such a change would break all tests. We call this problem a _fragile tests problem_. diff --git a/doc/development/testing_guide/end_to_end/quick_start_guide.md b/doc/development/testing_guide/end_to_end/quick_start_guide.md index e1df8be8b6f..d52d6db38b9 100644 --- a/doc/development/testing_guide/end_to_end/quick_start_guide.md +++ b/doc/development/testing_guide/end_to_end/quick_start_guide.md @@ -10,7 +10,7 @@ It's important to understand that end-to-end tests of isolated features, such as If you don't exactly understand what we mean by **not everything needs to happen through the GUI,** please make sure you've read the [best practices](best_practices.md) before moving on. -## This document covers the following items: +## This document covers the following items - [0.](#0-are-end-to-end-tests-needed) Identifying if end-to-end tests are really needed - [1.](#1-identifying-the-devops-stage) Identifying the [DevOps stage](https://about.gitlab.com/stages-devops-lifecycle/) of the feature that you are going to cover with end-to-end tests diff --git a/doc/development/testing_guide/end_to_end/style_guide.md b/doc/development/testing_guide/end_to_end/style_guide.md index 97560e616a1..54ed3f34c89 100644 --- a/doc/development/testing_guide/end_to_end/style_guide.md +++ b/doc/development/testing_guide/end_to_end/style_guide.md @@ -141,4 +141,4 @@ Resource::MergeRequest.fabricate! do |merge_request_page| end ``` -> Besides the advantage of having a standard in place, by following this standard we also write shorter lines of code.
\ No newline at end of file +> Besides the advantage of having a standard in place, by following this standard we also write shorter lines of code. diff --git a/doc/development/testing_guide/flaky_tests.md b/doc/development/testing_guide/flaky_tests.md index 931cbc51cae..eb0bf6fc563 100644 --- a/doc/development/testing_guide/flaky_tests.md +++ b/doc/development/testing_guide/flaky_tests.md @@ -35,8 +35,8 @@ Once a test is in quarantine, there are 3 choices: Quarantined tests are run on the CI in dedicated jobs that are allowed to fail: -- `rspec-pg-quarantine` and `rspec-mysql-quarantine` (CE & EE) -- `rspec-pg-quarantine-ee` and `rspec-mysql-quarantine-ee` (EE only) +- `rspec-pg-quarantine` (CE & EE) +- `rspec-pg-quarantine-ee` (EE only) ## Automatic retries and flaky tests detection diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 2985278cc92..7dc89a3fcdb 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -232,7 +232,7 @@ module. GitLab has a custom `spyOnDependency` method which utilizes [babel-plugin-rewire](https://github.com/speedskater/babel-plugin-rewire) to achieve this. It can be used like so: -```js +```javascript // my_module.js import { visitUrl } from '~/lib/utils/url_utility'; @@ -241,7 +241,7 @@ export default function doSomething() { } ``` -```js +```javascript // my_module_spec.js import doSomething from '~/my_module'; @@ -593,6 +593,517 @@ end [capybara]: https://github.com/teamcapybara/capybara [jasmine]: https://jasmine.github.io/ +## Overview of Frontend Testing Levels + +Tests relevant for frontend development can be found at the following places: + +- `spec/javascripts/` which are run by Karma (command: `yarn karma`) and contain + - [frontend unit tests](#frontend-unit-tests) + - [frontend component tests](#frontend-component-tests) + - [frontend integration tests](#frontend-integration-tests) +- `spec/frontend/` which are run by Jest (command: `yarn jest`) and contain + - [frontend unit tests](#frontend-unit-tests) + - [frontend component tests](#frontend-component-tests) + - [frontend integration tests](#frontend-integration-tests) +- `spec/features/` which are run by RSpec and contain + - [feature tests](#feature-tests) + +All tests in `spec/javascripts/` will eventually be migrated to `spec/frontend/` (see also [#52483](https://gitlab.com/gitlab-org/gitlab-ce/issues/52483)). + +In addition, there used to be feature tests in `features/`, run by Spinach. +These were removed from the codebase in May 2018 ([#23036](https://gitlab.com/gitlab-org/gitlab-ce/issues/23036)). + +See also [Notes on testing Vue components](../fe_guide/vue.html#testing-vue-components). + +### Frontend unit tests + +Unit tests are on the lowest abstraction level and typically test functionality that is not directly perceivable by a user. + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class plain tested; + class Vuex tested; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use unit tests + +<details> + <summary>exported functions and classes</summary> + Anything that is exported can be reused at various places in a way you have no control over. + Therefore it is necessary to document the expected behavior of the public interface with tests. +</details> + +<details> + <summary>Vuex actions</summary> + Any Vuex action needs to work in a consistent way independent of the component it is triggered from. +</details> + +<details> + <summary>Vuex mutations</summary> + For complex Vuex mutations it helps to identify the source of a problem by separating the tests from other parts of the Vuex store. +</details> + +#### When *not* to use unit tests + +<details> + <summary>non-exported functions or classes</summary> + Anything that is not exported from a module can be considered private or an implementation detail and doesn't need to be tested. +</details> + +<details> + <summary>constants</summary> + Testing the value of a constant would mean to copy it. + This results in extra effort without additional confidence that the value is correct. +</details> + +<details> + <summary>Vue components</summary> + Computed properties, methods, and lifecycle hooks can be considered an implementation detail of components and don't need to be tested. + They are implicitly covered by component tests. + The <a href="https://vue-test-utils.vuejs.org/guides/#getting-started">official Vue guidelines</a> suggest the same. +</details> + +#### What to mock in unit tests + +<details> + <summary>state of the class under test</summary> + Modifying the state of the class under test directly rather than using methods of the class avoids side-effects in test setup. +</details> + +<details> + <summary>other exported classes</summary> + Every class needs to be tested in isolation to prevent test scenarios from growing exponentially. +</details> + +<details> + <summary>single DOM elements if passed as parameters</summary> + For tests that only operate on single DOM elements rather than a whole page, creating these elements is cheaper than loading a whole HTML fixture. +</details> + +<details> + <summary>all server requests</summary> + When running frontend unit tests, the backend may not be reachable. + Therefore all outgoing requests need to be mocked. +</details> + +<details> + <summary>asynchronous background operations</summary> + Background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. +</details> + +#### What *not* to mock in unit tests + +<details> + <summary>non-exported functions or classes</summary> + Everything that is not exported can be considered private to the module and will be implicitly tested via the exported classes / functions. +</details> + +<details> + <summary>methods of the class under test</summary> + By mocking methods of the class under test, the mocks will be tested and not the real methods. +</details> + +<details> + <summary>utility functions (pure functions, or those that only modify parameters)</summary> + If a function has no side effects because it has no state, it is safe to not mock it in tests. +</details> + +<details> + <summary>full HTML pages</summary> + Loading the HTML of a full page slows down tests, so it should be avoided in unit tests. +</details> + +### Frontend component tests + +Component tests cover the state of a single component that is perceivable by a user depending on external signals such as user input, events fired from other components, or application state. + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class Vue tested; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use component tests + +- Vue components + +#### When *not* to use component tests + +<details> + <summary>Vue applications</summary> + Vue applications may contain many components. + Testing them on a component level requires too much effort. + Therefore they are tested on frontend integration level. +</details> + +<details> + <summary>HAML templates</summary> + HAML templates contain only Markup and no frontend-side logic. + Therefore they are not complete components. +</details> + +#### What to mock in component tests + +<details> + <summary>DOM</summary> + Operating on the real DOM is significantly slower than on the virtual DOM. +</details> + +<details> + <summary>properties and state of the component under test</summary> + Similarly to testing classes, modifying the properties directly (rather than relying on methods of the component) avoids side-effects. +</details> + +<details> + <summary>Vuex store</summary> + To avoid side effects and keep component tests simple, Vuex stores are replaced with mocks. +</details> + +<details> + <summary>all server requests</summary> + Similar to unit tests, when running component tests, the backend may not be reachable. + Therefore all outgoing requests need to be mocked. +</details> + +<details> + <summary>asynchronous background operations</summary> + Similar to unit tests, background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. +</details> + +<details> + <summary>child components</summary> + Every component is tested individually, so child components are mocked. + See also <a href="https://vue-test-utils.vuejs.org/api/#shallowmount">shallowMount()</a> +</details> + +#### What *not* to mock in component tests + +<details> + <summary>methods or computed properties of the component under test</summary> + By mocking part of the component under test, the mocks will be tested and not the real component. +</details> + +<details> + <summary>functions and classes independent from Vue</summary> + All plain JavaScript code is already covered by unit tests and needs not to be mocked in component tests. +</details> + +### Frontend integration tests + +Integration tests cover the interaction between all components on a single page. +Their abstraction level is comparable to how a user would interact with the UI. + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class plain tested; + class Vue tested; + class Vuex tested; + class GraphQL tested; + class browser tested; + linkStyle 0,1,2,3,4,5,6 stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use integration tests + +<details> + <summary>page bundles (<code>index.js</code> files in <code>app/assets/javascripts/pages/</code>)</summary> + Testing the page bundles ensures the corresponding frontend components integrate well. +</details> + +<details> + <summary>Vue applications outside of page bundles</summary> + Testing Vue applications as a whole ensures the corresponding frontend components integrate well. +</details> + +#### What to mock in integration tests + +<details> + <summary>HAML views (use fixtures instead)</summary> + Rendering HAML views requires a Rails environment including a running database which we cannot rely on in frontend tests. +</details> + +<details> + <summary>all server requests</summary> + Similar to unit and component tests, when running component tests, the backend may not be reachable. + Therefore all outgoing requests need to be mocked. +</details> + +<details> + <summary>asynchronous background operations that are not perceivable on the page</summary> + Background operations that affect the page need to be tested on this level. + All other background operations cannot be stopped or waited on, so they will continue running in the following tests and cause side effects. +</details> + +#### What *not* to mock in integration tests + +<details> + <summary>DOM</summary> + Testing on the real DOM ensures our components work in the environment they are meant for. + Part of this will be delegated to <a href="https://gitlab.com/gitlab-org/quality/team-tasks/issues/45">cross-browser testing</a>. +</details> + +<details> + <summary>properties or state of components</summary> + On this level, all tests can only perform actions a user would do. + For example to change the state of a component, a click event would be fired. +</details> + +<details> + <summary>Vuex stores</summary> + When testing the frontend code of a page as a whole, the interaction between Vue components and Vuex stores is covered as well. +</details> + +### Feature tests + +In contrast to [frontend integration tests](#frontend-integration-tests), feature tests make requests against the real backend instead of using fixtures. +This also implies that database queries are executed which makes this category significantly slower. + +See also the [RSpec testing guidelines](../testing_guide/best_practices.md#rspec). + +```mermaid +graph RL + plain[Plain JavaScript]; + Vue[Vue Components]; + feature-flags[Feature Flags]; + license-checks[License Checks]; + + plain---Vuex; + plain---GraphQL; + Vue---plain; + Vue---Vuex; + Vue---GraphQL; + browser---plain; + browser---Vue; + plain---backend; + Vuex---backend; + GraphQL---backend; + Vue---backend; + backend---database; + backend---feature-flags; + backend---license-checks; + + class backend tested; + class plain tested; + class Vue tested; + class Vuex tested; + class GraphQL tested; + class browser tested; + linkStyle 0,1,2,3,4,5,6,7,8,9,10 stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + classDef node color:#909090,fill:#f0f0f0,stroke-width:2px,stroke:#909090 + classDef label stroke-width:0; + classDef tested color:#000000,fill:#a0c0ff,stroke:#6666ff,stroke-width:2px,stroke-dasharray: 5, 5; + + subgraph " " + tested; + mocked; + class tested tested; + end +``` + +#### When to use feature tests + +- Use cases that require a backend and cannot be tested using fixtures. +- Behavior that is not part of a page bundle but defined globally. + +#### Relevant notes + +A `:js` flag is added to the test to make sure the full environment is loaded. + +```ruby +scenario 'successfully', :js do + sign_in(create(:admin)) +end +``` + +The steps of each test are written using capybara methods ([documentation](https://www.rubydoc.info/gems/capybara)). + +Bear in mind <abbr title="XMLHttpRequest">XHR</abbr> calls might require you to use `wait_for_requests` in between steps, like so: + +```ruby +find('.form-control').native.send_keys(:enter) + +wait_for_requests + +expect(page).not_to have_selector('.card') +``` + +## Test helpers + +### Vuex Helper: `testAction` + +We have a helper available to make testing actions easier, as per [official documentation](https://vuex.vuejs.org/guide/testing.html): + +```javascript +testAction( + actions.actionName, // action + { }, // params to be passed to action + state, // state + [ + { type: types.MUTATION}, + { type: types.MUTATION_1, payload: {}}, + ], // mutations committed + [ + { type: 'actionName', payload: {}}, + { type: 'actionName1', payload: {}}, + ] // actions dispatched + done, +); +``` + +Check an example in [spec/javascripts/ide/stores/actions_spec.jsspec/javascripts/ide/stores/actions_spec.js](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/ide/stores/actions_spec.js). + +### Vue Helper: `mountComponent` + +To make mounting a Vue component easier and more readable, we have a few helpers available in `spec/helpers/vue_mount_component_helper`: + +- `createComponentWithStore` +- `mountComponentWithStore` + +Examples of usage: + +```javascript +beforeEach(() => { + vm = createComponentWithStore(Component, store); + + vm.$store.state.currentBranchId = 'master'; + + vm.$mount(); +}); +``` + +```javascript +beforeEach(() => { + vm = mountComponentWithStore(Component, { + el: '#dummy-element', + store, + props: { badge }, + }); +}); +``` + +Don't forget to clean up: + +```javascript +afterEach(() => { + vm.$destroy(); +}); +``` + +## Testing with older browsers + +Some regressions only affect a specific browser version. We can install and test in particular browsers with either Firefox or Browserstack using the following steps: + +### Browserstack + +[Browserstack](https://www.browserstack.com/) allows you to test more than 1200 mobile devices and browsers. +You can use it directly through the [live app](https://www.browserstack.com/live) or you can install the [chrome extension](https://chrome.google.com/webstore/detail/browserstack/nkihdmlheodkdfojglpcjjmioefjahjb) for easy access. +You can find the credentials on 1Password, under `frontendteam@gitlab.com`. + +### Firefox + +#### macOS + +You can download any older version of Firefox from the releases FTP server, <https://ftp.mozilla.org/pub/firefox/releases/>: + +1. From the website, select a version, in this case `50.0.1`. +1. Go to the mac folder. +1. Select your preferred language, you will find the dmg package inside, download it. +1. Drag and drop the application to any other folder but the `Applications` folder. +1. Rename the application to something like `Firefox_Old`. +1. Move the application to the `Applications` folder. +1. Open up a terminal and run `/Applications/Firefox_Old.app/Contents/MacOS/firefox-bin -profilemanager` to create a new profile specific to that Firefox version. +1. Once the profile has been created, quit the app, and run it again like normal. You now have a working older Firefox version. + --- [Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md index 96e8c30a679..173471e3af8 100644 --- a/doc/development/testing_guide/index.md +++ b/doc/development/testing_guide/index.md @@ -13,7 +13,7 @@ importance. GitLab is built on top of [Ruby on Rails](https://rubyonrails.org/), and we're using [RSpec] for all the backend tests, with [Capybara] for end-to-end integration testing. -On the frontend side, we're using [Karma] and [Jasmine] for JavaScript unit and +On the frontend side, we're using [Jest](https://jestjs.io/) and [Karma](http://karma-runner.github.io/)/[Jasmine](https://jasmine.github.io/) for JavaScript unit and integration testing. Following are two great articles that everyone should read to understand what @@ -64,6 +64,4 @@ Everything you should know about how to run end-to-end tests using [RSpec]: https://github.com/rspec/rspec-rails#feature-specs [Capybara]: https://github.com/teamcapybara/capybara -[Karma]: http://karma-runner.github.io/ -[Jasmine]: https://jasmine.github.io/ [gitlab-qa]: https://gitlab.com/gitlab-org/gitlab-qa diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 7843fc4c874..28a60660995 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -132,7 +132,7 @@ to prevent other pods from being scheduled on this node pool. This is to ensure Tiller isn't affected by "noisy" neighbors that could put their node under pressure. -## How to: +## How to ### Log into my Review App @@ -255,8 +255,8 @@ that a machine will hit the "too many mount points" problem in the future. thousands of unused Docker images.** > We have to start somewhere and improve later. Also, we're using the - CNG-mirror project to store these Docker images so that we can just wipe out - the registry at some point, and use a new fresh, empty one. + > CNG-mirror project to store these Docker images so that we can just wipe out + > the registry at some point, and use a new fresh, empty one. **How do we secure this from abuse? Apps are open to the world so we need to find a way to limit it to only us.** diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index e1ce4d3b7d1..1aee306f492 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -63,10 +63,9 @@ They're useful to test permissions, redirections, what view is rendered etc. | Code path | Tests path | Testing engine | Notes | | --------- | ---------- | -------------- | ----- | -| `app/controllers/` | `spec/controllers/` | RSpec | | +| `app/controllers/` | `spec/controllers/` | RSpec | For N+1 tests, use [request specs](../query_recorder.md#use-request-specs-instead-of-controller-specs) | | `app/mailers/` | `spec/mailers/` | RSpec | | | `lib/api/` | `spec/requests/api/` | RSpec | | -| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | | `app/assets/javascripts/` | `spec/javascripts/`, `spec/frontend/` | Karma & Jest | More details in the [Frontend Testing guide](frontend_testing.md) section. | ### About controller tests @@ -127,7 +126,7 @@ possible). | ---------- | -------------- | ----- | | `spec/features/` | [Capybara] + [RSpec] | If your test has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. | -### Consider **not** writing a system test! +### Consider **not** writing a system test If we're confident that the low-level components work well (and we should be if we have enough Unit & Integration tests), we shouldn't need to duplicate their diff --git a/doc/development/uploads.md b/doc/development/uploads.md new file mode 100644 index 00000000000..681ce9d9fe8 --- /dev/null +++ b/doc/development/uploads.md @@ -0,0 +1,270 @@ +# Uploads development documentation + +[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse) has special rules for handling uploads. +To prevent occupying a ruby process on I/O operations, we process the upload in workhorse, where is cheaper. +This process can also directly upload to object storage. + +## The problem description + +The following graph explains machine boundaries in a scalable GitLab installation. Without any workhorse optimization in place, we can expect incoming requests to follow the numbers on the arrows. + +```mermaid +graph TB + subgraph "load balancers" + LB(HA Proxy) + end + + subgraph "Shared storage" + nfs(NFS) + end + + subgraph "redis cluster" + r(persisted redis) + end + LB-- 1 -->workhorse + + subgraph "web or API fleet" + workhorse-- 2 -->rails + end + rails-- "3 (write files)" -->nfs + rails-- "4 (schedule a job)" -->r + + subgraph sidekiq + s(sidekiq) + end + s-- "5 (fetch a job)" -->r + s-- "6 (read files)" -->nfs +``` + +We have three challenges here: performance, availability, and scalability. + +### Performance + +Rails process are expensive in terms of both CPU and memory. Ruby [global interpreter lock](https://en.wikipedia.org/wiki/Global_interpreter_lock) adds to cost too because the ruby process will spend time on I/O operations on step 3 causing incoming requests to pile up. + +In order to improve this, [workhorse disk acceleration](#workhorse-disk-acceleration) was implemented. With this, Rails no longer deals with writing uploaded files to disk. + +```mermaid +graph TB + subgraph "load balancers" + LB(HA Proxy) + end + + subgraph "Shared storage" + nfs(NFS) + end + + subgraph "redis cluster" + r(persisted redis) + end + LB-- 1 -->workhorse + + subgraph "web or API fleet" + workhorse-- "3 (without files)" -->rails + end + workhorse -- "2 (write files)" -->nfs + rails-- "4 (schedule a job)" -->r + + subgraph sidekiq + s(sidekiq) + end + s-- "5 (fetch a job)" -->r + s-- "6 (read files)" -->nfs +``` + +### Availability + +There's also an availability problem in this setup, NFS is a [single point of failure](https://en.wikipedia.org/wiki/Single_point_of_failure). + +To address this problem an HA object storage can be used and it's supported by [workhorse object storage acceleration](#workhorse-object-storage-acceleration) + +### Scalability + +Scaling NFS is outside of our support scope, and NFS is not a part of cloud native installations. + +All features that require sidekiq and do not use object storage acceleration won't work without NFS. In Kubernetes, machine boundaries translate to PODs, and in this case the uploaded file will be written into the POD private disk. Since sidekiq POD cannot reach into other pods, the operation will fail to read it. + +## How to select the proper level of acceleration? + +Selecting the proper acceleration is a tradeoff between speed of development and operational costs. + +We can identify three major use-cases for an upload: + +1. **storage:** if we are uploading for storing a file (i.e. artifacts, packages, discussion attachments). In this case [object storage acceleration](#workhorse-object-storage-acceleration) is the proper level as it's the less resource-intensive operation. Additional information can be found on [File Storage in GitLab](file_storage.md). +1. **in-controller/synchronous processing:** if we allow processing **small files** synchronously, using [disk acceleration](#workhorse-disk-acceleration) may speed up development. +1. **sidekiq/asynchronous processing:** Async processing must implement [object storage acceleration](#workhorse-object-storage-acceleration), the reason being that it's the only way to support Cloud Native deployments without a shared NFS. + +For more details about currently broken feature see [epic &1802](https://gitlab.com/groups/gitlab-org/-/epics/1802). + +### Handling repository uploads + +Some features involves git repository uploads without using a regular git client. +Some examples are uploading a repository file from the web interface and [design management](../user/project/issues/design_management.md). + +Those uploads requires the rails controller to act as a git client in lieu of the user. +Those operation falls into _in-controller/synchronous processing_ category, but we have no warranties on the file size. + +In case of a LFS upload, the file pointer is committed synchronously, but file upload to object storage is performed asynchronously with sidekiq. + +## Upload encodings + +By upload encoding we mean how the file is included within the incoming request. + +We have three kinds of file encoding in our uploads: + +1. <i class="fa fa-check-circle"></i> **multipart**: `multipart/form-data` is the most common, a file is encoded as a part of a multipart encoded request. +1. <i class="fa fa-check-circle"></i> **body**: some APIs uploads files as the whole request body. +1. <i class="fa fa-times-circle"></i> **JSON**: some JSON API uploads files as base64 encoded strings. This requires [gitlab-workhorse#226](https://gitlab.com/gitlab-org/gitlab-workhorse/issues/226) to be implemented. + +## Uploading technologies + +By uploading technologies we mean how all the involved services interact with each other. + +GitLab supports 3 kinds of uploading technologies, here follows a brief description with a sequence diagram for each one. Diagrams are not meant to be exhaustive. + +### Regular rails upload + +This is the default kind of upload, and it's most expensive in terms of resources. + +In this case, workhorse is unaware of files being uploaded and acts as a regular proxy. + +When a multipart request reaches the rails application, `Rack::Multipart` leaves behind tempfiles in `/tmp` and uses valuable Ruby process time to copy files around. + +```mermaid +sequenceDiagram + participant c as Client + participant w as Workhorse + participant r as Rails + + activate c + c ->>+w: POST /some/url/upload + w->>+r: POST /some/url/upload + + r->>r: save the incoming file on /tmp + r->>r: read the file for processing + + r-->>-c: request result + deactivate c + deactivate w +``` + +### Workhorse disk acceleration + +This kind of upload avoids wasting resources caused by handling upload writes to `/tmp` in rails. + +This optimization is not active by default on REST API requests. + +When enabled, Workhorse looks for files in multipart MIME requests, uploading +any it finds to a temporary file on shared storage. The MIME data in the request +is replaced with the path to the corresponding file before it is forwarded to +Rails. + +To prevent abuse of this feature, Workhorse signs the modified request with a +special header, stating which entries it modified. Rails will ignore any +unsigned path entries. + +```mermaid +sequenceDiagram + participant c as Client + participant w as Workhorse + participant r as Rails + participant s as NFS + + activate c + c ->>+w: POST /some/url/upload + + w->>+s: save the incoming file on a temporary location + s-->>-w: + + w->>+r: POST /some/url/upload + Note over w,r: file was replaced with its location<br>and other metadata + + opt requires async processing + r->>+redis: schedule a job + redis-->>-r: + end + + r-->>-c: request result + deactivate c + w->>-w: cleanup + + opt requires async processing + activate sidekiq + sidekiq->>+redis: fetch a job + redis-->>-sidekiq: job + + sidekiq->>+s: read file + s-->>-sidekiq: file + + sidekiq->>sidekiq: process file + + deactivate sidekiq + end +``` + +### Workhorse object storage acceleration + +This is the more advanced acceleration technique we have in place. + +Workhorse asks rails for temporary pre-signed object storage URLs and directly uploads to object storage. + +In this setup an extra rails route needs to be implemented in order to handle authorization, +you can see an example of this in [`Projects::LfsStorageController`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/controllers/projects/lfs_storage_controller.rb) +and [its routes](https://gitlab.com/gitlab-org/gitlab-ce/blob/v12.2.0/config/routes/git_http.rb#L31-32). + +**note:** this will fallback to _Workhorse disk acceleration_ when object storage is not enabled in the gitlab instance. The answer to the `/authorize` call will only contain a file system path. + +```mermaid +sequenceDiagram + participant c as Client + participant w as Workhorse + participant r as Rails + participant os as Object Storage + + activate c + c ->>+w: POST /some/url/upload + + w ->>+r: POST /some/url/upload/authorize + Note over w,r: this request has an empty body + r-->>-w: presigned OS URL + + w->>+os: PUT file + Note over w,os: file is stored on a temporary location. Rails select the destination + os-->>-w: + + w->>+r: POST /some/url/upload + Note over w,r: file was replaced with its location<br>and other metadata + + r->>+os: move object to final destination + os-->>-r: + + opt requires async processing + r->>+redis: schedule a job + redis-->>-r: + end + + r-->>-c: request result + deactivate c + w->>-w: cleanup + + opt requires async processing + activate sidekiq + sidekiq->>+redis: fetch a job + redis-->>-sidekiq: job + + sidekiq->>+os: get object + os-->>-sidekiq: file + + sidekiq->>sidekiq: process file + + deactivate sidekiq + end +``` + +## What does the `direct_upload` setting mean? + +[Object storage setting](../administration/uploads.md#object-storage-settings) allows instance administators to enable `direct_upload`, this in an option that only affects the behavior of [workhorse object storage acceleration](#workhorse-object-storage-acceleration). + +This option affect the response to the `/authorize` call. When not enabled, the API response will not contain presigned URLs and workhorse will write the file the shared disk, on the path is provided by rails, acting like object storage was disabled. + +Once the request reachs rails, it will schedule an object storage upload as a sidekiq job. diff --git a/doc/development/ux_guide/animation.md b/doc/development/ux_guide/animation.md index a998ab74a96..0f7a24042bb 100644 --- a/doc/development/ux_guide/animation.md +++ b/doc/development/ux_guide/animation.md @@ -1,5 +1,5 @@ --- -redirect_to: 'https://design.gitlab.com/product-foundations/motion' +redirect_to: 'https://design.gitlab.com/product-foundations/motion/' --- -The content of this document was moved into the [GitLab Design System](https://design.gitlab.com/product-foundations/motion). +The content of this document was moved into the [GitLab Design System](https://design.gitlab.com/product-foundations/motion/). diff --git a/doc/development/ux_guide/illustrations.md b/doc/development/ux_guide/illustrations.md index 3592d25c95d..815f870f8c5 100644 --- a/doc/development/ux_guide/illustrations.md +++ b/doc/development/ux_guide/illustrations.md @@ -1,5 +1,5 @@ --- -redirect_to: 'https://design.gitlab.com/product-foundations/illustration' +redirect_to: 'https://design.gitlab.com/product-foundations/illustration/' --- -The content of this document was moved into the [GitLab Design System](https://design.gitlab.com/product-foundations/illustration). +The content of this document was moved into the [GitLab Design System](https://design.gitlab.com/product-foundations/illustration/). diff --git a/doc/development/verifying_database_capabilities.md b/doc/development/verifying_database_capabilities.md index ccec6f7d719..6b4995aebe2 100644 --- a/doc/development/verifying_database_capabilities.md +++ b/doc/development/verifying_database_capabilities.md @@ -1,15 +1,15 @@ # Verifying Database Capabilities -Sometimes certain bits of code may only work on a certain database and/or +Sometimes certain bits of code may only work on a certain database version. While we try to avoid such code as much as possible sometimes it is necessary to add database (version) specific behaviour. To facilitate this we have the following methods that you can use: -- `Gitlab::Database.postgresql?`: returns `true` if PostgreSQL is being used -- `Gitlab::Database.mysql?`: returns `true` if MySQL is being used +- `Gitlab::Database.postgresql?`: returns `true` if PostgreSQL is being used. + You can normally just assume this is the case. - `Gitlab::Database.version`: returns the PostgreSQL version number as a string - in the format `X.Y.Z`. This method does not work for MySQL + in the format `X.Y.Z`. This allows you to write code such as: diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index f0da1cc2ddc..f4cee410066 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -7,9 +7,8 @@ downtime. ## Adding Columns -On PostgreSQL you can safely add a new column to an existing table as long as it -does **not** have a default value. For example, this query would not require -downtime: +You can safely add a new column to an existing table as long as it does **not** +have a default value. For example, this query would not require downtime: ```sql ALTER TABLE projects ADD COLUMN random_value int; @@ -27,11 +26,6 @@ This requires updating every single row in the `projects` table so that indexes in a table. This in turn acquires enough locks on the table for it to effectively block any other queries. -As of MySQL 5.6 adding a column to a table is still quite an expensive -operation, even when using `ALGORITHM=INPLACE` and `LOCK=NONE`. This means -downtime _may_ be required when modifying large tables as otherwise the -operation could potentially take hours to complete. - Adding a column with a default value _can_ be done without requiring downtime when using the migration helper method `Gitlab::Database::MigrationHelpers#add_column_with_default`. This method works @@ -51,15 +45,12 @@ rule. The first step is to ignore the column in the application code. This is necessary because Rails caches the columns and re-uses this cache in various -places. This can be done by including the `IgnorableColumn` module into the -model, followed by defining the columns to ignore. For example, to ignore +places. This can be done by defining the columns to ignore. For example, to ignore `updated_at` in the User model you'd use the following: ```ruby -class User < ActiveRecord::Base - include IgnorableColumn - - ignore_column :updated_at +class User < ApplicationRecord + self.ignored_columns += %i[updated_at] end ``` @@ -70,8 +61,7 @@ column. Both these changes should be submitted in the same merge request. Once the changes from step 1 have been released & deployed you can set up a separate merge request that removes the ignore rule. This merge request can -simply remove the `ignore_column` line, and the `include IgnorableColumn` line -if no other `ignore_column` calls remain. +simply remove the `self.ignored_columns` line. ## Renaming Columns @@ -311,8 +301,7 @@ migrations](background_migrations.md#cleaning-up). ## Adding Indexes Adding indexes is an expensive process that blocks INSERT and UPDATE queries for -the duration. When using PostgreSQL one can work around this by using the -`CONCURRENTLY` option: +the duration. You can work around this by using the `CONCURRENTLY` option: ```sql CREATE INDEX CONCURRENTLY index_name ON projects (column_name); @@ -336,17 +325,9 @@ end Note that `add_concurrent_index` can not be reversed automatically, thus you need to manually define `up` and `down`. -When running this on PostgreSQL the `CONCURRENTLY` option mentioned above is -used. On MySQL this method produces a regular `CREATE INDEX` query. - -MySQL doesn't really have a workaround for this. Supposedly it _can_ create -indexes without the need for downtime but only for variable width columns. The -details on this are a bit sketchy. Since it's better to be safe than sorry one -should assume that adding indexes requires downtime on MySQL. - ## Dropping Indexes -Dropping an index does not require downtime on both PostgreSQL and MySQL. +Dropping an index does not require downtime. ## Adding Tables @@ -370,7 +351,7 @@ transaction this means this approach would require downtime. GitLab allows you to work around this by using `Gitlab::Database::MigrationHelpers#add_concurrent_foreign_key`. This method -ensures that when PostgreSQL is used no downtime is needed. +ensures that no downtime is needed. ## Removing Foreign Keys |
