Skip to content

Allocate IID for deployments outside of the pipeline transaction

Shinya Maeda requested to merge deployment-iid-transaction-improvement into master

What does this MR do?

Today, Deployment records are created in pipeline transaction (Ci::CreatePipelineService). This causes a problem that database COMMIT doesn't happen to internal_ids table during the long transaction, which could result in iid unique constraint violation and inefficient retry.

This MR builds Deployment objects and allocate iid before the pipeline transaction begins. This way, we can avoid this problem. You can find more context in #21518 (closed).

Close #21518 (closed)

Technical Details

First of all, we have Ci::CreatePipelineService. This is a single and comprehensive service to create a pipeline to a project. This service is called from bunch of places when the system needs to create a pipeline e.g. when user pushed a commit to a remote repository.

In this service, there are multiple steps to be executed sequentially for preparing, validating and inserting/updating database records. But in this MR's context, the important part is Gitlab::Ci::Pipeline::Chain::Populate and Gitlab::Ci::Pipeline::Chain::Create, which does the following things:

  • Chain::Populate ... Build a pipeline and the associations (e.g. Ci::Pipeline and Ci::Build objects) with proper attributes (read from .gitlab-ci.yml).
  • Chain::Create ... Create a pipeline and the associations i.e. Begins and commits a database transaction for a pipeline and inserts records into bunch of tables.

and in the current implementation, Deployment and Environment are built/created in Chain::Create (via Deployable concern). This is our problem to solve, that Deployment.has_internal_id is executed at this moment. It allocates an iid for the deployment and tries to update internal_ids.last_value but the transaction cannot be committed during the long Chain::Create step. This will result in that multiple Deployment has the same iid as internal_ids.last_value is not updated yet when multiple pipelines are created concurrently.

This MR resolves the problem by changing the process flow as the following:

  • Chain::Populate ... Build a pipeline and the associations (e.g. Ci::Pipeline, Ci::Build, Deployment and Environment objects) with proper attributes (read from .gitlab-ci.yml).
  • Chain::Create ... Create a pipeline and the associations i.e. Begins and commits a database transaction for a pipeline and inserts records into bunch of tables. Since Deployment object has already been built with allocated iid, the system doesn't need to re-allocate it.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] 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 Shinya Maeda

Merge request reports