Skip to content

Dont use DB connection in GlobalSearchSampler

What does this MR do?

As discovered in #231248 (closed) the GlobalSearchSampler which invokes this Elastic::MetricsUpdateService is using an extra DB connection since it runs in it's own thread. This extra DB connection is not accounted for and can cause issues with the connection pool.

We could account for this extra connection but it turns out the 2 database queries happening in here aren't really necessary anyway. The first checks if the Elasticsearch integration is enabled. Skipping this is safe since it will just set 0 for these metrics which is not a problem. The 2nd checks to see if prometheus monitoring is enabled. But this check was already happening before spawning the GlobalSearchSampler in config/initializers/7_prometheus_metrics.rb so we can safely skip that too.

The only other things happening now in this class are requests to Redis and don't require a DB connection.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability 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

Closes #231248 (closed)

Edited by Dylan Griffith

Merge request reports