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

Client side caching refactoring #3350

Merged
merged 89 commits into from
Sep 27, 2024
Merged

Conversation

vladvildanov
Copy link
Collaborator

@vladvildanov vladvildanov commented Aug 8, 2024

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

Refactor for existing CSC implementation. The core changes is:

  1. Single cache instance per connection pool. In RedisCluster single cache instance per all nodes.
  2. Caching logic now is on the level of connection, so it can be reusable by different clients.
  3. Additional abstractions around caching domain (CacheKey, CacheEntry, EvictionPolicy...)
  4. Increased test coverage.
  5. Added minimal version restrictions for the feature (>= Redis 7.4)

gerzse and others added 9 commits July 19, 2024 18:11
Right now the client side caching code is implemented mostly on the
level of Connections, which is too low. We need to have a shared cache
across several connections.

Move the cache implementation higher, while trying to encapsulate it
better, into a `CacheMixin` class.

This is work in progress, many details still need to be taken care of!
@vladvildanov vladvildanov force-pushed the client-side-caching-improvements branch from 01d82f5 to 2c50adc Compare August 9, 2024 14:06
@vladvildanov vladvildanov force-pushed the client-side-caching-improvements branch from edde84c to 64fb176 Compare August 15, 2024 14:30
Copy link
Contributor

@uglide uglide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my suggestions in the inline comments.

docs/resp3_features.rst Outdated Show resolved Hide resolved
docs/resp3_features.rst Outdated Show resolved Hide resolved
redis/cache.py Outdated Show resolved Hide resolved
redis/cache.py Outdated Show resolved Hide resolved
@vladvildanov vladvildanov merged commit 2e46613 into master Sep 27, 2024
55 checks passed
@vladvildanov vladvildanov deleted the client-side-caching-improvements branch September 27, 2024 11:16
vladvildanov added a commit that referenced this pull request Sep 27, 2024
* Restructure client side caching code

Right now the client side caching code is implemented mostly on the
level of Connections, which is too low. We need to have a shared cache
across several connections.

Move the cache implementation higher, while trying to encapsulate it
better, into a `CacheMixin` class.

This is work in progress, many details still need to be taken care of!

* Temporary refactor

* Finished CacheProxyConnection implementation, added comments

* Added test cases and scheduler dependency

* Added support for RedisCluster and multi-threaded test cases

* Added support for BlockingConnectionPool

* Fixed docker-compose command

* Revert port changes

* Initial take on Sentinel support

* Remove keys option after usage

* Added condition to remove keys entry on async

* Added same keys entry removal in pipeline

* Added caching support for Sentinel

* Added locking when accesing cache object

* Rmoved keys option from options

* Removed redundant entities

* Added cache support for SSLConnection

* Moved ssl argument handling to cover cluster case

* Revert local test changes

* Fixed bug with missing async operator

* Revert accidental changes

* Added API to return cache object

* Added eviction policy configuration

* Added mark to skip test on cluster

* Removed test case that makes no sense

* Skip tests in RESP2

* Added scheduler to dev_requirements

* Codestyle changes

* Fixed characters per line restriction

* Fixed line length

* Removed blank lines in imports

* Fixed imports codestyle

* Added CacheInterface abstraction

* Removed redundant references

* Moved hardcoded values to constants, restricted dependency versions

* Changed defaults to correct values

* Added custom background scheduler, added unit testing

* Codestyle changes

* Updated RESP2 restriction

* Cahnged typing to more generic

* Restrict pytest-asyncio version to 0.23

* Added upper version limit

* Removed usntable multithreaded tests

* Removed more flacky multithreaded tests

* Fixed issue with Sentinel killing healthcheck thread before execution

* Removed cachetools dependency, added custom cache implementation

* Updated test cases

* Updated typings

* Updated types

* Revert changes

* Removed use_cache, make health_check configurable, removed retry logic around can_read()

* Revert test skip

* Added documentation and codestyle fixes

* Updated excluded wordlist

* Added health_check thread cancelling in BlockingPool

* Revert argument rename, extended documentation

* Updated NodesManager to create shared cache between all nodes

* Codestyle fixes

* Updated docs

* Added version restrictions

* Added missing property getter

* Updated Redis server version

* Skip on long exception message

* Removed keys entry as it's csc specific

* Updated exception message for CSC

* Updated condition by adding server name check

* Added test coverage for decoded responses

* Codestyle changes

* Removed background healthcheck, use connection reference approach instead

* Removed unused imports

* Fixed broken tests

* Codestyle changes

* Fixed additional broken tests

* Codestyle changes

* Increased timer to avoid flackiness

* Restrict tests cause of PyPy

* Codestyle changes

* Updated docs, convert getters function to properties, added dataclasses

---------

Co-authored-by: Gabriel Erzse <[email protected]>
vladvildanov added a commit that referenced this pull request Sep 27, 2024
* Restructure client side caching code

Right now the client side caching code is implemented mostly on the
level of Connections, which is too low. We need to have a shared cache
across several connections.

Move the cache implementation higher, while trying to encapsulate it
better, into a `CacheMixin` class.

This is work in progress, many details still need to be taken care of!

* Temporary refactor

* Finished CacheProxyConnection implementation, added comments

* Added test cases and scheduler dependency

* Added support for RedisCluster and multi-threaded test cases

* Added support for BlockingConnectionPool

* Fixed docker-compose command

* Revert port changes

* Initial take on Sentinel support

* Remove keys option after usage

* Added condition to remove keys entry on async

* Added same keys entry removal in pipeline

* Added caching support for Sentinel

* Added locking when accesing cache object

* Rmoved keys option from options

* Removed redundant entities

* Added cache support for SSLConnection

* Moved ssl argument handling to cover cluster case

* Revert local test changes

* Fixed bug with missing async operator

* Revert accidental changes

* Added API to return cache object

* Added eviction policy configuration

* Added mark to skip test on cluster

* Removed test case that makes no sense

* Skip tests in RESP2

* Added scheduler to dev_requirements

* Codestyle changes

* Fixed characters per line restriction

* Fixed line length

* Removed blank lines in imports

* Fixed imports codestyle

* Added CacheInterface abstraction

* Removed redundant references

* Moved hardcoded values to constants, restricted dependency versions

* Changed defaults to correct values

* Added custom background scheduler, added unit testing

* Codestyle changes

* Updated RESP2 restriction

* Cahnged typing to more generic

* Restrict pytest-asyncio version to 0.23

* Added upper version limit

* Removed usntable multithreaded tests

* Removed more flacky multithreaded tests

* Fixed issue with Sentinel killing healthcheck thread before execution

* Removed cachetools dependency, added custom cache implementation

* Updated test cases

* Updated typings

* Updated types

* Revert changes

* Removed use_cache, make health_check configurable, removed retry logic around can_read()

* Revert test skip

* Added documentation and codestyle fixes

* Updated excluded wordlist

* Added health_check thread cancelling in BlockingPool

* Revert argument rename, extended documentation

* Updated NodesManager to create shared cache between all nodes

* Codestyle fixes

* Updated docs

* Added version restrictions

* Added missing property getter

* Updated Redis server version

* Skip on long exception message

* Removed keys entry as it's csc specific

* Updated exception message for CSC

* Updated condition by adding server name check

* Added test coverage for decoded responses

* Codestyle changes

* Removed background healthcheck, use connection reference approach instead

* Removed unused imports

* Fixed broken tests

* Codestyle changes

* Fixed additional broken tests

* Codestyle changes

* Increased timer to avoid flackiness

* Restrict tests cause of PyPy

* Codestyle changes

* Updated docs, convert getters function to properties, added dataclasses

---------

Co-authored-by: Gabriel Erzse <[email protected]>
vladvildanov added a commit that referenced this pull request Sep 27, 2024
* Restructure client side caching code

Right now the client side caching code is implemented mostly on the
level of Connections, which is too low. We need to have a shared cache
across several connections.

Move the cache implementation higher, while trying to encapsulate it
better, into a `CacheMixin` class.

This is work in progress, many details still need to be taken care of!

* Temporary refactor

* Finished CacheProxyConnection implementation, added comments

* Added test cases and scheduler dependency

* Added support for RedisCluster and multi-threaded test cases

* Added support for BlockingConnectionPool

* Fixed docker-compose command

* Revert port changes

* Initial take on Sentinel support

* Remove keys option after usage

* Added condition to remove keys entry on async

* Added same keys entry removal in pipeline

* Added caching support for Sentinel

* Added locking when accesing cache object

* Rmoved keys option from options

* Removed redundant entities

* Added cache support for SSLConnection

* Moved ssl argument handling to cover cluster case

* Revert local test changes

* Fixed bug with missing async operator

* Revert accidental changes

* Added API to return cache object

* Added eviction policy configuration

* Added mark to skip test on cluster

* Removed test case that makes no sense

* Skip tests in RESP2

* Added scheduler to dev_requirements

* Codestyle changes

* Fixed characters per line restriction

* Fixed line length

* Removed blank lines in imports

* Fixed imports codestyle

* Added CacheInterface abstraction

* Removed redundant references

* Moved hardcoded values to constants, restricted dependency versions

* Changed defaults to correct values

* Added custom background scheduler, added unit testing

* Codestyle changes

* Updated RESP2 restriction

* Cahnged typing to more generic

* Restrict pytest-asyncio version to 0.23

* Added upper version limit

* Removed usntable multithreaded tests

* Removed more flacky multithreaded tests

* Fixed issue with Sentinel killing healthcheck thread before execution

* Removed cachetools dependency, added custom cache implementation

* Updated test cases

* Updated typings

* Updated types

* Revert changes

* Removed use_cache, make health_check configurable, removed retry logic around can_read()

* Revert test skip

* Added documentation and codestyle fixes

* Updated excluded wordlist

* Added health_check thread cancelling in BlockingPool

* Revert argument rename, extended documentation

* Updated NodesManager to create shared cache between all nodes

* Codestyle fixes

* Updated docs

* Added version restrictions

* Added missing property getter

* Updated Redis server version

* Skip on long exception message

* Removed keys entry as it's csc specific

* Updated exception message for CSC

* Updated condition by adding server name check

* Added test coverage for decoded responses

* Codestyle changes

* Removed background healthcheck, use connection reference approach instead

* Removed unused imports

* Fixed broken tests

* Codestyle changes

* Fixed additional broken tests

* Codestyle changes

* Increased timer to avoid flackiness

* Restrict tests cause of PyPy

* Codestyle changes

* Updated docs, convert getters function to properties, added dataclasses

---------

Co-authored-by: Gabriel Erzse <[email protected]>
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