Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify cache redis backend #11452

Closed
wants to merge 1 commit into from
Closed

Modify cache redis backend #11452

wants to merge 1 commit into from

Conversation

pantaovay
Copy link

Changes

  1. Modify cache redis backend, delete _PHCR
  2. Cache redis backend supports "redis" client option

Why

  1. The redis cache backend use a special key _PHCR to store all keys used by cache. There is a issue: why the backend cache redis add the special key(statsKey) _PHCR #10905 .According to @Green-Cat ,it is disabled in 2.0.x branch which is not true. If use _PHCR, every time we set a cache, we have to run sAdd and set commands. Store all keys in _PHCR will cause it expands quickly and this will influence performance.
  2. The cache backend doesn't support redis option which means that we can not pass a redis connection. So if we use modelsCache and modelsMetadata, there will be two redis connections. So I modify this part and let redis option be supported, and through the lifetime of a request we can use one redis connection.

2. Cache redis backend supports "redis" client option
@pantaovay
Copy link
Author

@ReenExe @sergeyklay

@sergeyklay
Copy link
Contributor

Looks good to me

@andresgutierrez

@tylercb
Copy link

tylercb commented Mar 12, 2016

+1

1 similar comment
@Green-Cat
Copy link
Contributor

👍

@sergeyklay
Copy link
Contributor

@pantaovay 2.1.x is best place for this. Also could you please update CHANGELOG.md?

@pantaovay
Copy link
Author

@sergeyklay I am working on it

@sergeyklay sergeyklay added this to the 2.1.0 milestone Mar 20, 2016
@sergeyklay
Copy link
Contributor

@pantaovay

@sergeyklay sergeyklay removed this from the 2.1.0 milestone Apr 3, 2016
@sergeyklay
Copy link
Contributor

Close in favor of #11608

@sergeyklay sergeyklay closed this Apr 3, 2016
@niden niden mentioned this pull request Nov 1, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants