Skip to content

Upgrade redis gem v5 - 2nd attempt

Sylvester Chin requested to merge revert-3a4b5753 into master

What does this MR do and why?

This MR reverts the revert MR for !144224 (merged).

Following investigations in gitlab-com/gl-infra/scalability#2826 (closed), we have narrowed the root cause to the peek redis views which patches Redis::Client. The fix for this was merged separately in !144388 (merged).

There is an upstreamed fix in https://github.com/redis-rb/redis-client/pull/173 to prevent the command prelude from being mutated by its callers.

In this MR we also:

  • perform a minor clean up to reduce scope of the MR since lib/gitlab/redis/command_builder.rb handles NilTypes for us
  • omit the custom command builder from Sidekiq to avoid any

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

  1. Set up a local redis with password required and a short timeout of 5s
redis-server --port 6370 --requirepass foobar --timeout 5
  1. Update config.yml to use the
➜  gitlab git:(sc1-migrate-repository-cache) cat config/redis.yml
---
development:
  shared_state:
    url: 'redis://:foobar@localhost:6370'
  1. Start up gdk and open the web UI. Refresh after >5s. You should not see any errors.

  2. Use this commit 13e55430 and repeat step 3. Run gdk restart rails to see Redis::CannotConnectError at /flightjs/Flight WRONGPASS invalid username-password pair or user is disabled.

Screenshot_2024-02-13_at_1.36.23_PM

Edited by Sylvester Chin

Merge request reports