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

Slow delete_pattern() #609

Closed
jdbit opened this issue Jun 8, 2022 · 6 comments
Closed

Slow delete_pattern() #609

jdbit opened this issue Jun 8, 2022 · 6 comments

Comments

@jdbit
Copy link

jdbit commented Jun 8, 2022

Hi, I have a problem with delete_pattern() function.
This code with cache.delete_pattern() is very-very slow:

cache.delete_pattern("template.cache.some_cache*")

But when I rewrite the code, it works blazing fast:

keys = cache.keys("template.cache.some_cache*")
for o in keys:
    cache.delete(o)

Why the delete_pattern() is so slow? Is it a bug or I'm doing something wrong?

@jdbit jdbit added the bug label Jun 8, 2022
@WisdomPill
Copy link
Member

Hey @jdbit sorry for replying late.

Well the issue is that cache.delete_pattern uses cache.scan_iter, which internally uses SCAN command and by default return 10 items at a time, so actually if cache.delete_pattern does not have itersize it deletes 10 items at a time.

On the other hand cache.keys, returns all the items and the you actually delete 1 item at a time, I would suggest to use cache.delete_many which will be even faster.

Finally, probably client.scan_iter return 10 items much slower than deleting 10 items one at a time.

P.S. I did not verify that, it is just my suspicion.

@WisdomPill
Copy link
Member

Personally I would change cache.delete_pattern, to use keys and then delete every key using a redis pipeline, since I am not sure how many deletes could be done in one DEL command. Would you be interested in contributing and adding this feature under a feature flag? (like by adding a pipeline=False argument)

@jdbit
Copy link
Author

jdbit commented Jun 14, 2022

Hey @WisdomPill,
thank you for your reply. I just tried delete_many() and it works faster, thank you for the suggestion! Sorry, I'm a beginner, so no idea how to work with the redis pipeline. I'll try to learn from the code in this repository, maybe I can figure out how it works. Thank you!

@rootart
Copy link
Contributor

rootart commented Jul 2, 2022

@WisdomPill I would be happy to try it and implement the suggested feature. There are some concerns when using keys command for databases with many keys as it blocks other clients and could lead to performance issues. Would it be fine to have it turned off by default as a safe option?

@WisdomPill
Copy link
Member

@rootart you are right. There could be a safe way to use scan and a bigger iter_size but delete keys in one go and a more dangerous option using keys. Technically with a big enough iter_size it would get close to keys performance wise

rootart pushed a commit to rootart/django-redis that referenced this issue Jul 5, 2022
@rootart
Copy link
Contributor

rootart commented Jul 15, 2022

Hey @WisdomPill
Will appreciate it if you could have a quick look at the pr I prepared. Many thanks!

rootart pushed a commit to rootart/django-redis that referenced this issue Jul 23, 2022
WisdomPill pushed a commit that referenced this issue Oct 21, 2022
* #609, use pipeline for delete_pattern

* #609, add changelog description
@WisdomPill WisdomPill unpinned this issue Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants