-
Notifications
You must be signed in to change notification settings - Fork 23.6k
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
Fix dictScan(): It can't scan all buckets when dict is shrinking. #4907
Conversation
Thanks for this submission. Before looking at this patch I invoke the spirit of @pietern that wrote the original code, because I will be more comfortable changing this code (that so much helped Redis) after his nod :-D |
Thanks @youjiali1995, I've checked the problem, verified that your solution is sounding, and created a simulation that allows us to study SCAN better in case of new problems (https://github.com/antirez/dict-scan-fuzz-tester). I'm merging your Pull Request in all the branches. Please note that, fortunately, only SCAN is affected. HSCAN, ZSCAN and SSCAN should be immune to the problem because their dictionaries always expand from a smaller to a bigger one. The issue you found only affects dictScan() when going to smaller hash tables. Thank you for your contribution! |
Just one final thing @youjiali1995, I would love to know how you have found this bug. |
Because we use scan to clear data. But sometimes there are some keys left, and we can reproduce this problem. So I read the code and find the bug. @antirez |
Thanks, awesome process to find the bug. I appreciate that instead of just posting an issue you went the extra mile and read the code to understand what was happening. |
Fix dictScan(): It can't scan all buckets when dict is shrinking.
@youjiali1995 咨询你个问题:字典扫描都是从小表开始,游标从零开始,缩容的时候,参考你们博客https://tech.meituan.com/2018/07/27/redis-rehash-practice-optimization.html 游标返回的范围不应该是小表的范围吗? 看博客是示例应该0-7,为什么会出现返回20为游标的情况呢? (我知道如果人工指定游标参数的话会出现漏key情况) |
@beijingzhangwei You should preferably use English to describe the problem. |
@youjiali1995 Refer to your company blog https://tech.meituan.com/2018/07/27/redis-rehash-practice-optimization.html |
ok,i use english. My question come from @youjiali1995 company blog, which describe the bug. |
@beijingzhangwei Not sure I fully understand what you meant. |
Thx. I get it.
发自我的iPhone
…------------------ Original ------------------
From: sundb ***@***.***>
Date: Wed,Nov 17,2021 0:05 PM
To: redis/redis ***@***.***>
Cc: beijingzw ***@***.***>, Mention ***@***.***>
Subject: Re: [redis/redis] Fix dictScan(): It can't scan all buckets when dict is shrinking. (#4907)
@beijingzhangwei Not sure I fully understand what you meant.
Cursor 20 was fetched before rehashing, so returning 20 is possible.
After that, the cursor will be 0-7.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
|
Fix dictScan(): It can't scan all buckets when dict is shrinking.
The only difference is when iterating over larger table, cursor is increased in reverse:
When jumping out of the loop, the highest bit of (v & m0) is increased, so move following code to if block: