Skip to content

Prepare CI minutes consumption update to be idempotent

Fabio Pitino requested to merge prepare-ci-minutes-update-as-idempotent into master

What does this MR do?

Related to #331785 (closed)

When a job completes, BuildFinishedWorker runs and in turns schedules other workers, including Ci::Minutes::UpdateProjectAndNamespaceUsageWorker. This worker is responsible for incrementing the CI minutes consumption. It's necessary for this worker to be idempotent to avoid counting multiple times CI minutes consumption (equivalent to billing the customer multiple times for service).

In order to make Ci::Minutes::UpdateProjectAndNamespaceUsageWorker to be idempotent we need to:

  • differentiate parameters uniquely by adding a new build_id parameter in a multi-step deployment.
  • having unique set of params we can deduplicate jobs and mark the worker as idempotent
  • track any updates already occurred for the same job (we are using Redis for that) and skip if already done

In this MR we are introducing a new parameter build_id (being nil by default) and a mechanism that uses Redis to check if the same update has already occurrent in the past 24 hours. Since this worker is scheduled by BuildFinishedWorker, there could be chances that BuildFinishedWorker fails and is retried, causing Ci::Minutes::UpdateProjectAndNamespaceUsageWorker to be also retried. 24 hours should be enough time to prevent duplicates. Also, given that BuildFinishedWorker is scheduled once when a build completes, it should not happen that a build finishes again in the future.

In a follow-up MR (to be merged after this is deployed) we will make build_id to always be passed as parameter. Finally in a 3rd MR we will remove the default value nil as per https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#multi-step-deployment.

Screenshots or Screencasts (strongly suggested)

How to setup and validate locally (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Fabio Pitino

Merge request reports

Loading