Commit 2ebb0be6 authored by Bob Van Landuyt's avatar Bob Van Landuyt Committed by Marcia Ramos

Update the sidekiq-styleguide based on feedback

Addresses the feedback from https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23995
parent a9895a77
......@@ -401,6 +401,8 @@ default weight, which is 1.
## Worker context
> - [Introduced](https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/9) in GitLab 12.8.
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).
......@@ -417,27 +419,27 @@ need to do anything.
There are however some instances when there would be no context
present when the job is scheduled, or the context that is present is
likely to be incorrect. For these instances we've added rubocop-rules
likely to be incorrect. For these instances, we've added Rubocop rules
to draw attention and avoid incorrect metadata in our logs.
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, please leave a code-comment
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.
When you do provide objects to the context, please make sure that the
route for namespaces and projects is pre-loaded. This can be done using
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
the `.with_route` scope defined on all `Routable`s.
### Cron-Workers
### Cron workers
The context is automatically cleared for workers in the cronjob-queue
(which `include CronjobQueue`), even when scheduling them from
The context is automatically cleared for workers in the Cronjob queue
(`include CronjobQueue`), even when scheduling them from
requests. We do this to avoid incorrect metadata when other jobs are
scheduled from the cron-worker.
scheduled from the cron worker.
Cron-Workers themselves run instance wide, so they aren't scoped to
Cron workers themselves run instance wide, so they aren't scoped to
users, namespaces, projects, or other resources that should be added to
the context.
......@@ -449,46 +451,46 @@ somewhere within the worker:
1. Wrap the code that schedules jobs in the `with_context` helper:
```ruby
def perform
deletion_cutoff = Gitlab::CurrentSettings
.deletion_adjourned_period.days.ago.to_date
projects = Project.with_route.with_namespace
.aimed_for_deletion(deletion_cutoff)
```ruby
def perform
deletion_cutoff = Gitlab::CurrentSettings
.deletion_adjourned_period.days.ago.to_date
projects = Project.with_route.with_namespace
.aimed_for_deletion(deletion_cutoff)
projects.find_each(batch_size: 100).with_index do |project, index|
delay = index * INTERVAL
projects.find_each(batch_size: 100).with_index do |project, index|
delay = index * INTERVAL
with_context(project: project) do
AdjournedProjectDeletionWorker.perform_in(delay, project.id)
end
end
end
```
with_context(project: project) do
AdjournedProjectDeletionWorker.perform_in(delay, project.id)
end
end
end
```
1. Use the a batch scheduling method that provides context:
```ruby
def schedule_projects_in_batch(projects)
ProjectImportScheduleWorker.bulk_perform_async_with_contexts(
projects,
arguments_proc: -> (project) { project.id },
context_proc: -> (project) { { project: project } }
)
end
```
or when scheduling with delays:
```ruby
diffs.each_batch(of: BATCH_SIZE) do |diffs, index|
DeleteDiffFilesWorker
.bulk_perform_in_with_contexts(index * 5.minutes,
diffs,
arguments_proc: -> (diff) { diff.id },
context_proc: -> (diff) { { project: diff.merge_request.target_project } })
end
```
```ruby
def schedule_projects_in_batch(projects)
ProjectImportScheduleWorker.bulk_perform_async_with_contexts(
projects,
arguments_proc: -> (project) { project.id },
context_proc: -> (project) { { project: project } }
)
end
```
Or, when scheduling with delays:
```ruby
diffs.each_batch(of: BATCH_SIZE) do |diffs, index|
DeleteDiffFilesWorker
.bulk_perform_in_with_contexts(index * 5.minutes,
diffs,
arguments_proc: -> (diff) { diff.id },
context_proc: -> (diff) { { project: diff.merge_request.target_project } })
end
```
### Jobs scheduled in bulk
......@@ -512,11 +514,11 @@ For example:
Each object from the enumerable in the first argument is yielded into 2
blocks:
The `arguments_proc` which needs to return the list of arguments the
job needs to be scheduled with.
- The `arguments_proc` which needs to return the list of arguments the
job needs to be scheduled with.
The `context_proc` which needs to return a hash with the context
information for the job.
- The `context_proc` which needs to return a hash with the context
information for the job.
## Arguments logging
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment