-
Notifications
You must be signed in to change notification settings - Fork 734
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
Make KEYS to be an exact match if there is no pattern #792
Conversation
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #792 +/- ##
============================================
+ Coverage 70.25% 70.28% +0.03%
============================================
Files 112 112
Lines 60590 60607 +17
============================================
+ Hits 42567 42600 +33
+ Misses 18023 18007 -16
|
Maybe we can extend this more common, for something like |
it seems like a good idea in some engine, but i think it adds too much code, and it doesn't seem worth doing so much pre-optimization in here. |
We discussed this issue when implementing the We implemented this feature on our platform before (as early as version 2.8), and it resulted in very undesirable outcomes. Specifically, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. For instance, if a user initially used a non-pattern-based pattern like I believe the right thing to do is to persuade users to use |
ok, i did not read redis/redis#12536 at that time so i missed this context, sorry about that, and sorry i am not realizing it
so you guys revert it at the end? i can see it will cause trouble, a guy execute very fast in here, and then get blocked when executing somewhere else. But i think in this case, it is ok if we document it (or we can even not to document it), O(N) degenerates to O(1) which is OK in my opinion, the worst case is O(N) and we did not deny it, and if he get blocked at the end, it is also normal since it is a wrong case.
i agree with this actually, i hope we can do it. But there are sill a lot of user doing this in the wrong way, they did not bother to do the right thing and everytime they are blaming the "service", all we can do i think is at least try our best to migrate the impact with a small cost. But i do agree with this, i am ok with both (revert it or document it or just leave it alone), lets seeks a vote and then i can process it. And speaking of this, i would also like ask everyone to discuss this one: #866 (comment). What can or what should we do to migrate this. If we agree with something, i think we can fit it into 8.0 |
When I develop a new feature or optimization, I often remind myself that Redis and Valkey are products designed for online services. As an engineer at a cloud provider, with my experience facing various different scenarios, I have found that online services and offline services are quite different. The characteristic of online services is that they require a high level of determinism, both in terms of execution results and performance, because online services can change at any time due to user actions at the front end, while the deployed code for online services remains fixed. Therefore, uncertainty can potentially bring disaster to the business. Take the |
In valkey-io#792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts valkey-io#792. Signed-off-by: Binbin <binloveplay1314@qq.com>
In #792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts #792. Signed-off-by: Binbin <binloveplay1314@qq.com>
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com>
In #792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts #792. Signed-off-by: Binbin <binloveplay1314@qq.com>
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com>
In #792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts #792. Signed-off-by: Binbin <binloveplay1314@qq.com>
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
…io#964) In valkey-io#792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts valkey-io#792. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
…io#964) In valkey-io#792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts valkey-io#792. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
…io#964) In valkey-io#792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts valkey-io#792. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
Although KEYS is a dangerous command and we recommend people to avoid using it, some people who are not familiar with it still using it, and even use KEYS with no pattern at all. Once KEYS is using with no pattern, we can convert it to an exact match to avoid iterating over all data. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
…io#964) In valkey-io#792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts valkey-io#792. Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Ping Xie <pingxie@google.com>
Although KEYS is a dangerous command and we recommend people
to avoid using it, some people who are not familiar with it
still using it, and even use KEYS with no pattern at all.
Once KEYS is using with no pattern, we can convert it to an
exact match to avoid iterating over all data.
This PR got revert in #964.