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

Refactor Redis driver to read all keys efficiently #913

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mapcentia
Copy link

The Redis driver's function to read all keys has been streamlined. The change is adding a loop to scan and merge the keys iteratively. This ensures that the function will work correctly, even when the number of keys scanned exceeds the MAX_ALL_KEYS_COUNT.

Proposed changes

This is a fix for #912

Types of changes

What types of changes does your code introduce to Phpfastcache?

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves an existing code/behavior)
  • Deprecated third party dependency update
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation/Typo/Resource update that does not involve any code modification

Agreement

I have read the CONTRIBUTING and CODING GUIDELINE docs

The Redis driver's function to read all keys has been streamlined. The changes include removing the check for iterable and adding a loop to scan and merge the keys iteratively. This ensures that the function will work correctly, even when the number of keys retrieved exceeds the MAX_ALL_KEYS_COUNT.
@Geolim4
Copy link
Member

Geolim4 commented Jun 8, 2024

The Redis driver's function to read all keys has been streamlined.

Where did you see that ? From which version ?
Backward compatibility must be ensured if this is a very recent change of Redis extension.

@mapcentia
Copy link
Author

mapcentia commented Jun 10, 2024

I'm sorry for the not so good PR comment. By Redis driver's function to read all keys I mean the method driverReadAllKeys of lib/Phpfastcache/Drivers/Redis/Driver.php

@Geolim4
Copy link
Member

Geolim4 commented Jun 10, 2024

I just read this in the docs:

It is important to note that the MATCH filter is applied after elements are retrieved from the collection, just before returning data to the client. This means that if the pattern matches very little elements inside the collection, SCAN will likely return no elements in most iterations. An example is shown below:

I know understand the purpose of this PR. I'll check it asap.

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.

None yet

2 participants