diff options
Diffstat (limited to 'doc/development/sidekiq_style_guide.md')
| -rw-r--r-- | doc/development/sidekiq_style_guide.md | 97 |
1 files changed, 37 insertions, 60 deletions
diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 13ae39997bc..e4f07f732cf 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Sidekiq Style Guide @@ -27,7 +27,7 @@ After adding a new queue, run `bin/rake gitlab:sidekiq:all_queues_yml:generate` to regenerate `app/workers/all_queues.yml` or `ee/app/workers/all_queues.yml` so that it can be picked up by -[`sidekiq-cluster`](../administration/operations/extra_sidekiq_processes.md). +[`sidekiq-cluster`](../administration/operations/extra_sidekiq_processes.md). Additionally, run `bin/rake gitlab:sidekiq:sidekiq_queues_yml:generate` to regenerate `config/sidekiq_queues.yml`. @@ -42,8 +42,8 @@ without needing to explicitly list all their queue names. If, for example, all workers that are managed by `sidekiq-cron` use the `cronjob` queue namespace, we can spin up a Sidekiq process specifically for these kinds of scheduled jobs. If a new worker using the `cronjob` namespace is added later on, the Sidekiq -process will automatically pick up jobs for that worker too (after having been -restarted), without the need to change any configuration. +process also picks up jobs for that worker (after having been restarted), +without the need to change any configuration. A queue namespace can be set using the `queue_namespace` DSL class method: @@ -57,19 +57,19 @@ class SomeScheduledTaskWorker end ``` -Behind the scenes, this will set `SomeScheduledTaskWorker.queue` to -`cronjob:some_scheduled_task`. Commonly used namespaces will have their own +Behind the scenes, this sets `SomeScheduledTaskWorker.queue` to +`cronjob:some_scheduled_task`. Commonly used namespaces have their own concern module that can easily be included into the worker class, and that may set other Sidekiq options besides the queue namespace. `CronjobQueue`, for example, sets the namespace, but also disables retries. -`bundle exec sidekiq` is namespace-aware, and will automatically listen on all +`bundle exec sidekiq` is namespace-aware, and listens on all queues in a namespace (technically: all queues prefixed with the namespace name) when a namespace is provided instead of a simple queue name in the `--queue` (`-q`) option, or in the `:queues:` section in `config/sidekiq_queues.yml`. Note that adding a worker to an existing namespace should be done with care, as -the extra jobs will take resources away from jobs from workers that were already +the extra jobs 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. @@ -121,9 +121,8 @@ As a general rule, a worker can be considered idempotent if: A good example of that would be a cache expiration worker. -A job scheduled for an idempotent worker will automatically be -[deduplicated](#deduplication) when an unstarted job with the same -arguments is already in the queue. +A job scheduled for an idempotent worker is [deduplicated](#deduplication) when +an unstarted job with the same arguments is already in the queue. ### Ensuring a worker is idempotent @@ -160,9 +159,8 @@ end It's encouraged to only have the `idempotent!` call in the top-most worker class, even if the `perform` method is defined in another class or module. -If the worker class is not marked as idempotent, a cop will fail. -Consider skipping the cop if you're not confident your job can safely -run multiple times. +If the worker class isn't marked as idempotent, a cop fails. Consider skipping +the cop if you're not confident your job can safely run multiple times. ### Deduplication @@ -230,7 +228,7 @@ end #### Scheduling jobs in the future GitLab doesn't skip jobs scheduled in the future, as we assume that -the state will have changed by the time the job is scheduled to +the state has changed by the time the job is scheduled to execute. Deduplication of jobs scheduled in the feature is possible for both `until_executed` and `until_executing` strategies. @@ -251,26 +249,6 @@ module AuthorizedProjectUpdate end ``` -#### Troubleshooting - -If the automatic deduplication were to cause issues in certain -queues. This can be temporarily disabled by enabling a feature flag -named `disable_<queue name>_deduplication`. For example to disable -deduplication for the `AuthorizedProjectsWorker`, we would enable the -feature flag `disable_authorized_projects_deduplication`. - -From ChatOps: - -```shell -/chatops run feature set disable_authorized_projects_deduplication true -``` - -From the rails console: - -```ruby -Feature.enable!(:disable_authorized_projects_deduplication) -``` - ## Limited capacity worker It is possible to limit the number of concurrent running jobs for a worker class @@ -278,10 +256,10 @@ by using the `LimitedCapacity::Worker` concern. The worker must implement three methods: -- `perform_work` - the concern implements the usual `perform` method and calls -`perform_work` if there is any capacity available. -- `remaining_work_count` - number of jobs that will have work to perform. -- `max_running_jobs` - maximum number of jobs allowed to run concurrently. +- `perform_work`: The concern implements the usual `perform` method and calls + `perform_work` if there's any available capacity. +- `remaining_work_count`: Number of jobs that have work to perform. +- `max_running_jobs`: Maximum number of jobs allowed to run concurrently. ```ruby class MyDummyWorker @@ -303,7 +281,7 @@ end Additional to the regular worker, a cron worker must be defined as well to backfill the queue with jobs. the arguments passed to `perform_with_capacity` -will be passed along to the `perform_work` method. +are passed to the `perform_work` method. ```ruby class ScheduleMyDummyCronWorker @@ -318,10 +296,10 @@ end ### How many jobs are running? -It will be running `max_running_jobs` at almost all times. +It runs `max_running_jobs` at almost all times. -The cron worker will check the remaining capacity on each execution and it -will schedule at most `max_running_jobs` jobs. Those jobs on completion will +The cron worker checks the remaining capacity on each execution and it +schedules at most `max_running_jobs` jobs. Those jobs on completion re-enqueue themselves immediately, but not on failure. The cron worker is in charge of replacing those failed jobs. @@ -329,11 +307,11 @@ charge of replacing those failed jobs. This concern disables Sidekiq retries, logs the errors, and sends the job to the dead queue. This is done to have only one source that produces jobs and because -the retry would occupy a slot with a job that will be performed in the distant future. +the retry would occupy a slot with a job to perform in the distant future. We let the cron worker enqueue new jobs, this could be seen as our retry and back off mechanism because the job might fail again if executed immediately. -This means that for every failed job, we will be running at a lower capacity +This means that for every failed job, we run at a lower capacity until the cron worker fills the capacity again. If it is important for the worker not to get a backlog, exceptions must be handled in `#perform_work` and the job should not raise. @@ -416,7 +394,7 @@ each of which represents a particular type of workload. When changing a queue's urgency, or adding a new queue, we need to take into account the expected workload on the new shard. Note that, if we're changing an existing queue, there is also an effect on the old shard, -but that will always be a reduction in work. +but that always reduces work. To do this, we want to calculate the expected increase in total execution time and RPS (throughput) for the new shard. We can get these values from: @@ -429,7 +407,7 @@ and RPS (throughput) for the new shard. We can get these values from: - The [Shard Detail dashboard](https://dashboards.gitlab.net/d/sidekiq-shard-detail/sidekiq-shard-detail) has Total Execution Time and Throughput (RPS). The Shard Utilization - panel will show if there is currently any excess capacity for this + panel displays if there is currently any excess capacity for this shard. We can then calculate the RPS * average runtime (estimated for new jobs) @@ -454,7 +432,7 @@ Most background jobs in the GitLab application communicate with other GitLab services. For example, PostgreSQL, 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 +However, some jobs are dependent on external services in order to complete successfully. Some examples include: 1. Jobs which call web-hooks configured by a user. @@ -492,7 +470,7 @@ A job cannot be both high urgency and have external dependencies. 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 +Most workers tend to spend most of their time blocked, waiting on network responses from other services such as Redis, PostgreSQL, and Gitaly. Since Sidekiq is a multi-threaded environment, these jobs can be scheduled with high concurrency. @@ -505,8 +483,8 @@ 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 +If many threads are attempting to run Ruby code simultaneously, this leads +to contention on the GIL which has the effect of slowing down all processes. In high-traffic environments, knowing that a worker is CPU-bound allows us to @@ -517,9 +495,9 @@ 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 +10-50ms. This has an impact on the latency requirements for the worker. For this reason, `memory` bound, `urgency :high` jobs are not -permitted and will fail CI. In general, `memory` bound workers are +permitted and fail CI. In general, `memory` bound workers are discouraged, and alternative approaches to processing the work should be considered. @@ -583,12 +561,11 @@ default weight, which is 1. To have some more information about workers in the logs, we add [metadata to the jobs in the form of an `ApplicationContext`](logging.md#logging-context-metadata-through-rails-or-grape-requests). -In most cases, when scheduling a job from a request, this context will -already be deducted from the request and added to the scheduled -job. +In most cases, when scheduling a job from a request, this context is already +deducted from the request and added to the scheduled job. When a job runs, the context that was active when it was scheduled -will be restored. This causes the context to be propagated to any job +is restored. This causes the context to be propagated to any job scheduled from within the running job. All this means that in most cases, to add context to jobs, we don't @@ -603,7 +580,7 @@ As with most our cops, there are perfectly valid reasons for disabling them. In this case it could be that the context from the request is correct. Or maybe you've specified a context already in a way that isn't picked up by the cops. In any case, leave a code comment -pointing to which context will be used when disabling the cops. +pointing to which context to use when disabling the cops. When you do provide objects to the context, make sure that the route for namespaces and projects is pre-loaded. This can be done by using @@ -699,7 +676,7 @@ blocks: ## Arguments logging -As of GitLab 13.6, Sidekiq job arguments will be logged by default, unless [`SIDEKIQ_LOG_ARGUMENTS`](../administration/troubleshooting/sidekiq.md#log-arguments-to-sidekiq-jobs) +As of GitLab 13.6, Sidekiq job arguments are logged by default, unless [`SIDEKIQ_LOG_ARGUMENTS`](../administration/troubleshooting/sidekiq.md#log-arguments-to-sidekiq-jobs) is disabled. By default, the only arguments logged are numeric arguments, because @@ -820,7 +797,7 @@ This approach requires multiple releases. ##### Parameter hash -This approach will not require multiple releases if an existing worker already +This approach doesn't require multiple releases if an existing worker already uses a parameter hash. 1. Use a parameter hash in the worker to allow future flexibility. |