Add latest changes from gitlab-org/gitlab@master

4 files changed, 224 insertions, 27 deletions

diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index 421b70bd2db..a19f40d1824 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -368,7 +368,7 @@ Enterprise Edition instance. This has some implications:
       - [Background migrations](background_migrations.md) run in Sidekiq, and
         should only be done for migrations that would take an extreme amount of
         time at GitLab.com scale.
-1. **Sidekiq workers** [cannot change in a backwards-incompatible way](sidekiq_style_guide.md#removing-or-renaming-queues):
+1. **Sidekiq workers** [cannot change in a backwards-incompatible way](sidekiq_style_guide.md#sidekiq-compatibility-across-updates):
    1. Sidekiq queues are not drained before a deploy happens, so there will be
       workers in the queue from the previous version of GitLab.
    1. If you need to change a method signature, try to do so across two releases,
diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md
index d52a3e652e3..e433691c1ed 100644
--- a/doc/development/sidekiq_style_guide.md
+++ b/doc/development/sidekiq_style_guide.md
@@ -61,6 +61,168 @@ the extra jobs will take resources away from jobs from workers that were already
 there, if the resources available to the Sidekiq process handling the namespace
 are not adjusted appropriately.
 
+## Latency Sensitive Jobs
+
+If a large number of background jobs get scheduled at once, queueing of jobs may
+occur while jobs wait for a worker node to be become available. This is normal
+and gives the system resilience by allowing it to gracefully handle spikes in
+traffic. Some jobs, however, are more sensitive to latency than others. Examples
+of these jobs include:
+
+1. A job which updates a merge request following a push to a branch.
+1. A job which invalidates a cache of known branches for a project after a push
+   to the branch.
+1. A job which recalculates the groups and projects a user can see after a
+   change in permissions.
+1. A job which updates the status of a CI pipeline after a state change to a job
+   in the pipeline.
+
+When these jobs are delayed, the user may perceive the delay as a bug: for
+example, they may push a branch and then attempt to create a merge request for
+that branch, but be told in the UI that the branch does not exist. We deem these
+jobs to be `latency_sensitive`.
+
+Extra effort is made to ensure that these jobs are started within a very short
+period of time after being scheduled. However, in order to ensure throughput,
+these jobs also have very strict execution duration requirements:
+
+1. The median job execution time should be less than 1 second.
+1. 99% of jobs should complete within 10 seconds.
+
+If a worker cannot meet these expectations, then it cannot be treated as a
+`latency_sensitive` worker: consider redesigning the worker, or splitting the
+work between two different workers, one with `latency_sensitive` code that
+executes quickly, and the other with non-`latency_sensitive`, which has no
+execution latency requirements (but also has lower scheduling targets).
+
+This can be summed up in the following table:
+
+| **Latency Sensitivity** | **Queue Scheduling Target** | **Execution Latency Requirement**   |
+|-------------------------|-----------------------------|-------------------------------------|
+| Not `latency_sensitive` | 1 minute                    | Maximum run time of 1 hour          |
+| `latency_sensitive`     | 100 milliseconds            | p50 of 1 second, p99 of 10 seconds  |
+
+To mark a worker as being `latency_sensitive`, use the
+`latency_sensitive_worker!` attribute, as shown in this example:
+
+```ruby
+class LatencySensitiveWorker
+  include ApplicationWorker
+
+  latency_sensitive_worker!
+
+  # ...
+end
+```
+
+## Jobs with External Dependencies
+
+Most background jobs in the GitLab application communicate with other GitLab
+services, eg Postgres, Redis, Gitaly and Object Storage. These are considered
+to be "internal" dependencies for a job.
+
+However, some jobs will be dependent on external services in order to complete
+successfully. Some examples include:
+
+1. Jobs which call web-hooks configured by a user.
+1. Jobs which deploy an application to a k8s cluster configured by a user.
+
+These jobs have "external dependencies". This is important for the operation of
+the background processing cluster in several ways:
+
+1. Most external dependencies (such as web-hooks) do not provide SLOs, and
+   therefore we cannot guarantee the execution latencies on these jobs. Since we
+   cannot guarantee execution latency, we cannot ensure throughput and
+   therefore, in high-traffic environments, we need to ensure that jobs with
+   external dependencies are separated from `latency_sensitive` jobs, to ensure
+   throughput on those queues.
+1. Errors in jobs with external dependencies have higher alerting thresholds as
+   there is a likelihood that the cause of the error is external.
+
+```ruby
+class ExternalDependencyWorker
+  include ApplicationWorker
+
+  # Declares that this worker depends on
+  # third-party, external services in order
+  # to complete successfully
+  worker_has_external_dependencies!
+
+  # ...
+end
+```
+
+NOTE: **Note:** Note that a job cannot be both latency sensitive and have
+external dependencies.
+
+## CPU-bound and Memory-bound Workers
+
+Workers that are constrained by CPU or memory resource limitations should be
+annotated with the `worker_resource_boundary` method.
+
+Most workers tend to spend most of their time blocked, wait on network responses
+from other services such as Redis, Postgres and Gitaly. Since Sidekiq is a
+multithreaded environment, these jobs can be scheduled with high concurrency.
+
+Some workers, however, spend large amounts of time _on-cpu_ running logic in
+Ruby. Ruby MRI does not support true multithreading - it relies on the
+[GIL](https://thoughtbot.com/blog/untangling-ruby-threads#the-global-interpreter-lock)
+to greatly simplify application development by only allowing one section of Ruby
+code in a process to run at a time, no matter how many cores the machine
+hosting the process has. For IO bound workers, this is not a problem, since most
+of the threads are blocked in underlying libraries (which are outside of the
+GIL).
+
+If many threads are attempting to run Ruby code simultaneously, this will lead
+to contention on the GIL which will have the affect of slowing down all
+processes.
+
+In high-traffic environments, knowing that a worker is CPU-bound allows us to
+run it on a different fleet with lower concurrency. This ensures optimal
+performance.
+
+Likewise, if a worker uses large amounts of memory, we can run these on a
+bespoke low concurrency, high memory fleet.
+
+Note that Memory-bound workers create heavy GC workloads, with pauses of
+10-50ms. This will have an impact on the latency requirements for the
+worker. For this reason, `memory` bound, `latency_sensitive` jobs are not
+permitted and will fail CI. In general, `memory` bound workers are
+discouraged, and alternative approaches to processing the work should be
+considered.
+
+## Declaring a Job as CPU-bound
+
+This example shows how to declare a job as being CPU-bound.
+
+```ruby
+class CPUIntensiveWorker
+  include ApplicationWorker
+
+  # Declares that this worker will perform a lot of
+  # calculations on-CPU.
+  worker_resource_boundary :cpu
+
+  # ...
+end
+```
+
+## Determining whether a worker is CPU-bound
+
+We use the following approach to determine whether a worker is CPU-bound:
+
+- In the sidekiq structured JSON logs, aggregate the worker `duration` and
+  `cpu_s` fields.
+- `duration` refers to the total job execution duration, in seconds
+- `cpu_s` is derived from the
+  [`Process::CLOCK_THREAD_CPUTIME_ID`](https://www.rubydoc.info/stdlib/core/Process:clock_gettime)
+  counter, and is a measure of time spent by the job on-CPU.
+- Divide `cpu_s` by `duration` to get the percentage time spend on-CPU.
+- If this ratio exceeds 33%, the worker is considered CPU-bound and should be
+  annotated as such.
+- Note that these values should not be used over small sample sizes, but
+  rather over fairly large aggregates.
+
 ## Feature Categorization
 
 Each Sidekiq worker, or one of its ancestor classes, must declare a
@@ -74,7 +236,7 @@ The declaration uses the `feature_category` class method, as shown below.
 class SomeScheduledTaskWorker
   include ApplicationWorker
 
-  # Declares that this feature is part of the
+  # Declares that this worker is part of the
   # `continuous_integration` feature category
   feature_category :continuous_integration
 
@@ -88,11 +250,11 @@ source](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml
 
 ### Updating `config/feature_categories.yml`
 
-Occassionally new features will be added to GitLab stages. When this occurs, you
+Occasionally new features will be added to GitLab stages. When this occurs, you
 can automatically update `config/feature_categories.yml` by running
 `scripts/update-feature-categories`. This script will fetch and parse
 [`stages.yml`](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml)
-and generare a new version of the file, which needs to be checked into source control.
+and generate a new version of the file, which needs to be checked into source control.
 
 ### Excluding Sidekiq workers from feature categorization
 
@@ -116,9 +278,63 @@ end
 Each Sidekiq worker must be tested using RSpec, just like any other class. These
 tests should be placed in `spec/workers`.
 
-## Removing or renaming queues
+## Sidekiq Compatibility across Updates
+
+Keep in mind that the arguments for a Sidekiq job are stored in a queue while it
+is scheduled for execution. During a online update, this could lead to several
+possible situations:
+
+1. An older version of the application publishes a job, which is executed by an
+   upgraded Sidekiq node.
+1. A job is queued before an upgrade, but executed after an upgrade.
+1. A job is queued by a node running the newer version of the application, but
+   executed on a node running an older version of the application.
+
+### Changing the arguments for a worker
+
+Jobs need to be backwards- and forwards-compatible between consecutive versions
+of the application.
+
+This can be done by following this process:
+
+1. **Do not remove arguments from the `perform` function.**. Instead, use the
+   following approach
+   1. Provide a default value (usually `nil`) and use a comment to mark the
+      argument as deprecated
+   1. Stop using the argument in `perform_async`.
+   1. Ignore the value in the worker class, but do not remove it until the next
+      major release.
+
+### Removing workers
+
+Try to avoid removing workers and their queues in minor and patch
+releases.
 
-Try to avoid renaming or removing workers and their queues in minor and patch releases.
 During online update instance can have pending jobs and removing the queue can
 lead to those jobs being stuck forever. If you can't write migration for those
-Sidekiq jobs, please consider doing rename or remove queue in major release only.
+Sidekiq jobs, please consider removing the worker in a major release only.
+
+### Renaming queues
+
+For the same reasons that removing workers is dangerous, care should be taken
+when renaming queues.
+
+When renaming queues, use the `sidekiq_queue_migrate` helper migration method,
+as show in this example:
+
+```ruby
+class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name'
+  end
+
+  def down
+    sidekiq_queue_migrate 'new_queue_name', to: 'old_queue_name'
+  end
+end
+
+```
diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md
index 270c6255350..27d6cc9b47b 100644
--- a/doc/topics/autodevops/index.md
+++ b/doc/topics/autodevops/index.md
@@ -969,7 +969,6 @@ The following table lists variables related to security tools.
 
 | **Variable**                            | **Description**                    |
 | `SAST_CONFIDENCE_LEVEL`                 | Minimum confidence level of security issues you want to be reported; `1` for Low, `2` for Medium, `3` for High. Defaults to `3`. |
-| `DS_DISABLE_REMOTE_CHECKS`              | Whether remote Dependency Scanning checks are disabled. Defaults to `"false"`. Set to `"true"` to disable checks that send data to GitLab central servers. [Read more about remote checks](../../user/application_security/dependency_scanning/index.md#remote-checks). |
 
 #### Disable jobs
 
diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md
index 9f87d79025e..15f6ded2587 100644
--- a/doc/user/application_security/dependency_scanning/index.md
+++ b/doc/user/application_security/dependency_scanning/index.md
@@ -63,23 +63,6 @@ The following languages and dependency managers are supported.
 | Python ([poetry](https://poetry.eustace.io/)) | not currently ([issue](https://gitlab.com/gitlab-org/gitlab/issues/7006 "Support Poetry in Dependency Scanning")) | not available |
 | Ruby ([gem](https://rubygems.org/)) | yes | [gemnasium](https://gitlab.com/gitlab-org/security-products/gemnasium), [bundler-audit](https://github.com/rubysec/bundler-audit) |
 
-## Remote checks
-
-While some tools pull a local database to check vulnerabilities, some others
-like Gemnasium require sending data to GitLab central servers to analyze them:
-
-1. Gemnasium scans the dependencies of your project locally and sends a list of
-   packages to GitLab central servers.
-1. The servers return the list of known vulnerabilities for all versions of
-   these packages.
-1. The client picks up the relevant vulnerabilities by comparing with the versions
-   of the packages that are used by the project.
-
-The Gemnasium client does **NOT** send the exact package versions your project relies on.
-
-You can disable the remote checks by [using](#customizing-the-dependency-scanning-settings)
-the `DS_DISABLE_REMOTE_CHECKS` environment variable and setting it to `"true"`.
-
 ## Configuration
 
 For GitLab 11.9 and later, to enable Dependency Scanning, you must
@@ -116,7 +99,7 @@ include:
   template: Dependency-Scanning.gitlab-ci.yml
 
 variables:
-  DS_DISABLE_REMOTE_CHECKS: "true"
+  DS_PYTHON_VERSION: 2
 ```
 
 Because template is [evaluated before](../../../ci/yaml/README.md#include) the pipeline
@@ -150,7 +133,6 @@ using environment variables.
 | `DS_PYTHON_VERSION`                     | Version of Python. If set to 2, dependencies are installed using Python 2.7 instead of Python 3.6. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12296) in GitLab 12.1)| |
 | `DS_PIP_DEPENDENCY_PATH`                | Path to load Python pip dependencies from. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12412) in GitLab 12.2) | |
 | `DS_DEFAULT_ANALYZERS`                  | Override the names of the official default images. Read more about [customizing analyzers](analyzers.md). | |
-| `DS_DISABLE_REMOTE_CHECKS`              | Do not send any data to GitLab. Used in the [Gemnasium analyzer](#remote-checks). | |
 | `DS_PULL_ANALYZER_IMAGES`               | Pull the images from the Docker registry (set to `0` to disable). | |
 | `DS_EXCLUDED_PATHS`                     | Exclude vulnerabilities from output based on the paths. A comma-separated list of patterns. Patterns can be globs, file or folder paths. Parent directories will also match patterns. | `DS_EXCLUDED_PATHS=doc,spec` |
 | `DS_DOCKER_CLIENT_NEGOTIATION_TIMEOUT`  | Time limit for Docker client negotiation. Timeouts are parsed using Go's [`ParseDuration`](https://golang.org/pkg/time/#ParseDuration). Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`. For example, `300ms`, `1.5h`, or `2h45m`. | |