Skip to content

Remove SIDEKIQ_REQUEST_STORE env variable

Bob Van Landuyt requested to merge bvl-remove-sidekiq-request-store-env into master

What does this MR do?

This removes the possibility to disable the request store in Sidekiq. The feature was enabled for a few months, and we don't know of anybody that actually used this ENV-flag.

It also prevents enabling a nested request store:

When calling #with_request_store we enable the request store and ensure it's disabled after the block completes.

However, since the request store is thread local, we can't enable and disable it twice withing the same block. In regular circumstances, this doesn't happen, since we only enable the request store in middleware, and they both run in a different process:

  • Using RequestStore::Middleware as a rack middleware for requests
  • Using Gitlab::SidekiqMiddleware::RequestStoreMiddleware for sidekiq-server.

In specs this could happen when running Sidekiq::Testing.inline!. If that was used within a spec that had the :request_store tag, or within a feature/request spec that ran the rack middleware.

In this case, the Sidekiq middleware would disable and clear the request store too soon, causing failures in the specs that counted on the request store, for example specs counting queries, or checking the number of Gitaly calls.

In specs, we work around this by making sure each spec has a clean request store to work with.

For this we needed to bump RequestStore to version 1.5, so it includes RequestStore.store =.

Closes to #214776 (closed)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Edited by Bob Van Landuyt

Merge request reports