NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.7k

[Cache] RedisTrait::doFetch should use pipeline with GET's instead of MGET for Relay\Cluster#61234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:6.4fromdorrogeray:cache-relay-cluster-should-use-pipeline-instead-of-mget-in-do-fetch

Jul 30, 2025

Merged

[Cache] RedisTrait::doFetch should use pipeline with GET's instead of MGET for Relay\Cluster#61234

nicolas-grekas merged 1 commit intosymfony:6.4fromdorrogeray:cache-relay-cluster-should-use-pipeline-instead-of-mget-in-do-fetch

Jul 30, 2025

Conversation

Copy link

Contributor

dorrogeray commentedJul 25, 2025•
edited by nicolas-grekas
Loading

Q	A
Branch?	6.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues	Fix #...
License	MIT

🛠️Relay\Cluster should be expected to operate with >1 master nodes, and therefore its not safe to usemget when fetching the ids inRedisTrait::doFetch(array $ids) as they are not guaranteed to be on a same hash slot.

This change makesdoFetch use pipeline withGET commands when$this->redis instanceof RelayCluster

carsonbot added Status: Needs Review Bug Cache labels

Jul 25, 2025

carsonbot added this to the7.3 milestone

Jul 25, 2025

carsonbot changed the title~~[cache] RedisTrait::doFetch should use pipeline with GET's instead of MGET for Relay\Cluster~~[Cache] RedisTrait::doFetch should use pipeline with GET's instead of MGET for Relay\Cluster

Jul 25, 2025

nicolas-grekas approved these changes

Jul 29, 2025

View reviewed changes

src/Symfony/Component/Cache/Traits/RedisTrait.php OutdatedShow resolvedHide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jul 29, 2025

dorrogeray force-pushed thecache-relay-cluster-should-use-pipeline-instead-of-mget-in-do-fetch branch fromf4fad81 tob74ec69Compare

July 30, 2025 09:25

Copy link

ContributorAuthor

dorrogeray commentedJul 30, 2025

@nicolas-grekas done, I have dropped the commit which applied the diff from Fabbot. 👍 The only reason I included them originally was to make those checks pass.