Fix 500 errors on commit lists when Redis runs in cluster mode

What does this MR do and why?

This bumps redis-cluster-client from 0.17.0 to 0.17.1 in Gemfile.lock and Gemfile.next.lock, and adds a >= 0.17.1 floor to the redis-cluster-client line in the Gemfile. There are no application code changes.

The floor is there because the lockfile alone does not stop a downgrade: redis-clustering only requires redis-cluster-client (>= 0.10.0), so under the old ~> 0.13 constraint the resolver could land back on 0.17.0, which is easy to miss in a lockfile diff. Keeping the constraint as '~> 0.13', '>= 0.17.1' rather than '~> 0.17.1' still leaves future minor versions such as 0.18 available. This two-constraint form is the pattern doc/development/gemfile.md prescribes, and matches existing entries like bcrypt and doorkeeper.

GitLab preloads cached rendered markdown for many commits at once on the commits list page and the compare page, using a single pipelined Redis command. When Redis runs in cluster mode and that pipeline hits a MOVED/ASK redirection on a node (during slot migration or failover), redis-cluster-client 0.17.0 skips the client-side reply transformation for every command sent to that node. mapped_hmget then returns a bare positional Array instead of a Hash. The markdown cache preloader destructures the Array as a Hash, assigns the rendered commit title to a field name, and calls instance_variable_set with it, which raises:

NameError: `@<commit title>' is not allowed as an instance variable name

Users saw a 500 whose message contained an actual commit title.

The upstream fix adds Pipeline::Extended#coerce_except!, which applies the transformation block to every reply on a redirected node except the redirected commands themselves. Those are re-sent to the correct node with their block attached, so they get coerced there. The same commit also adds a missing coercion call on the cluster-state-error path, which had the same bug.

A previous merge request fixed this with an application-level workaround in Gitlab::MarkdownCache::Redis::Store, sending a plain hmget and pairing the reply with the field list by hand. That approach was dropped: the underlying bug affects any pipelined command that relies on reply coercion, for example pipeline.exists? in Geo::ChecksumMismatchReportingService, not just the markdown cache. Hardening one call site would leave the rest exposed. The gem bump fixes all of them at once.

References

Screenshots or screen recordings

N/A — dependency update, no UI changes.

How to set up and validate locally

This change is lockfile-only, so there is nothing to exercise directly in the app. Reproducing the original failure requires a Redis Cluster undergoing slot migration, which is not practical to set up locally.

The practical checks, both run against this branch:

  • bundle exec rspec spec/lib/gitlab/markdown_cache/ — 56 examples, 0 failures.
  • bundle exec rake bundler:gemfile:check — both lockfiles report consistent.
  • The floor was checked against a lockfile pinned at 0.17.0: without it bundler keeps 0.17.0, with it bundler resolves 0.17.1.

MR acceptance checklist

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

Edited by Hordur Freyr Yngvason

Merge request reports

Loading
Loading