Skip to content

Fix logging method_missing commands in Redis::Multistore

Nikola Milojevic requested to merge 1438-add_missing_methods_to_multistore into master

What does this MR do and why?

Our gitlab_redis_multi_store_method_missing_total counter was triggered for 3 commands that are currently not supported by MultiStore:

  • info
  • incr
  • expire

  1. info command is used to show Redis version on the admin page

    • There is no point in falling back to the secondary store for the info command, it's safe to let the method_missing method handle it, and just omit it from the logs and counter.

  2. incr and expire are executed by anonymous_session.rb - When the feature flag use_primary_and_secondary_stores_for_sessions is enabled, the commands will be handeled by with_instance block, and the counter will not be incremented, which is not the case when the ff is disabled.

With explicitly using the block instance, like

- redis.pipelined do
+ redis.pipelined do |pipeline|
-   redis.incr(session_lookup_name)
+   pipeline.incr(session_lookup_name)
-   redis.expire(session_lookup_name, 24.hours)
+   pipeline.expire(session_lookup_name, 24.hours)
end

We don't have this issue anymore.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to gitlab-com/gl-infra/scalability#1438 (closed)

Edited by Nikola Milojevic

Merge request reports