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

Distributed cache feature using Redis #518

Merged
merged 22 commits into from
Aug 17, 2023

Conversation

a9raag
Copy link
Contributor

@a9raag a9raag commented Aug 16, 2023

Distributed Cache using Redis #496

Feature for distributed cache. This allows us to store keys in Redis key-value store.
Sorry, it took a while I spent too much time thinking about a design for this and how it will integrate into GPTCache's existing code base.

I have also attached a high-level design of how Distributed Cache works.

Notes

A couple of notes about how the eviction will work based on different scalar storage configurations:

Scenario 1:
Scalar Storage is MongoDB,
Eviction Base is Redis.
Then the keys will be stored in Redis, and cache call back will update the TTL.

Scenario 2:
Scalar Storage is Redis,
Eviction Base is also Redis,
Then storing keys separately becomes redundant since Redis eviction is based on Memory usage rather than the number of keys.
Also, we'll be fetching values multiple times both while getting scalar data and while hitting cache callback, and it will add network overhead.

To tackle this I created a NoOpEviction implementation (which skips cache operations) since,

  1. Redis internally manage the eviction based on memory size and eviction policy for all keys.
  2. We can configure RedisCacheStorage to update TTL upon creation and during access. This way values will be fetched from Redis only once.

GPT Distributed Cache

Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
@mergify mergify bot added the dco-passed label Aug 16, 2023
@SimFG SimFG changed the base branch from main to dev August 16, 2023 08:27
…rison

Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
@a9raag
Copy link
Contributor Author

a9raag commented Aug 16, 2023

@SimFG
Can you help the test cases are failing because of

'(ReadTimeoutError("HTTPSConnectionPool(host='huggingface.co', port=443): Read timed out. (read timeout=10.0)"),
 '(Request ID: 3afc7015-e7b6-4b6a-abd6-acd698b9469f)')' thrown while requesting GET 
https://huggingface.co/GPTCache/paraphrase-albert-small-v2/resolve/main/tokenizer.json

@SimFG
Copy link
Collaborator

SimFG commented Aug 16, 2023

@a9raag ok, I have rerun it

… to include memory cache config

Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
@a9raag
Copy link
Contributor Author

a9raag commented Aug 16, 2023

@SimFG it failed again

Signed-off-by: Anurag Wagh <a9raag@gmail.com>
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #518 (ce8db36) into dev (5aa7e1e) will decrease coverage by 0.16%.
Report is 1 commits behind head on dev.
The diff coverage is 93.33%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #518      +/-   ##
==========================================
- Coverage   94.05%   93.89%   -0.16%     
==========================================
  Files          94       95       +1     
  Lines        3920     4014      +94     
==========================================
+ Hits         3687     3769      +82     
- Misses        233      245      +12     
Files Changed Coverage Δ
gptcache/manager/eviction/memory_cache.py 90.90% <ø> (ø)
gptcache/manager/eviction/distributed_cache.py 89.65% <89.65%> (ø)
gptcache/manager/scalar_data/redis_storage.py 96.13% <93.18%> (-1.51%) ⬇️
gptcache/__init__.py 100.00% <100.00%> (ø)
gptcache/manager/data_manager.py 89.60% <100.00%> (+0.10%) ⬆️
gptcache/manager/eviction/manager.py 100.00% <100.00%> (ø)
gptcache/manager/factory.py 96.55% <100.00%> (+0.89%) ⬆️

... and 1 file with indirect coverage changes

Signed-off-by: Anurag Wagh <a9raag@gmail.com>
Signed-off-by: Anurag Wagh <a9raag@gmail.com>
…t_data_manager`

Signed-off-by: Anurag Wagh <a9raag@gmail.com>
@a9raag
Copy link
Contributor Author

a9raag commented Aug 17, 2023

@SimFG apologies if I keep bugging you but the Unit Test workflow failed again 😭

Here's the error log

/home/runner/work/_temp/c7cf298b-8f2b-475f-995a-f3f5562139f1.sh: line 3:  3496 Illegal instruction    
 (core dumped) python3 -m pytest -k "not embedding and not processor" --cov=gptcache --cov-report xml:coverage.xml --cov-append ./tests/
Error: Process completed with exit code 132.

@a9raag a9raag changed the title Feature/distributed cache Distributed cache feature using Redis Aug 17, 2023
@mergify mergify bot added the ci-passed label Aug 17, 2023
@a9raag
Copy link
Contributor Author

a9raag commented Aug 17, 2023

Hi @SimFG and @xiaofan-luan
I've updated the PR.
The Unit Tests and CodeCov have now passed.
Could you please review it when you get a chance?
Thanks : )

@SimFG
Copy link
Collaborator

SimFG commented Aug 17, 2023

@a9raag great!!! i will check it today

@SimFG
Copy link
Collaborator

SimFG commented Aug 17, 2023

/lgtm
/approve

@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a9raag, SimFG

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit c2deaec into zilliztech:dev Aug 17, 2023
8 checks passed
@a9raag a9raag deleted the feature/distributed-cache branch September 14, 2023 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants