-
Notifications
You must be signed in to change notification settings - Fork 653
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
[NEW] Support Check and Set functionality #1215
Comments
@zuiderkwast had already written up how the api might look like redis/redis#12485 (comment) Want to hear the reasoning @valkey-io/core-team if we would like to build it in Valkey or not. I personally feel we should build the API instead of pointing users to use LUA for basic use cases. |
From discussion redis/redis#8361, it looks like many people are waiting for this, I have no reason to against it. But I have one concern: if in production environment, if client connects the server by connection pool, this command may not work well. Is there a way to apply to connection pooling scenario? |
The proposal is really about data versioning and it makes sense on a high level, but stopping at SET/GET and numeric operations feels somewhat arbitrary and could lead to a fragmented or inconsistent experience. I'd like to explore why we're limiting versioning to just these cases. We should think about what it would mean to support other data types like hashes, sets, and streams. Even if we decide to start with just strings and numeric operations, it’s important to consciously consider how this might extend to other data types in the future to avoid potential gaps in functionality. It would be helpful to make a deliberate decision about the scope now, even if that means narrowing it for the initial implementation. Do you mind starting an RFC to capture a holistic proposal? |
Check-and-Set? (We're not really swapping two values like the compare-and-swap CPU instruction.) I see three ways to do this:
I don't think we want to store a version tag on each key. That would increase the memory usage and have other impacts, like RDB, AOF, etc. (On the other hand, such version tag or timestamp could be useful for other features like CRDTs with last-write-wins, active-active.) We already have CAS-like functionality in the SET XX and NX flags, and HSETNX for hash fields. Those are very simple and don't require anything special from the internals. That's why I only suggested simplistic variants for SET. Often, the CAS problem can be solved with a separate "cas" key which is used to guard updates on other keys. The "cas" key only needs to store some small user-defined tag. Therefore, I think SET with IF-EQ can already be very powerful.
For arbitrary key types and commands, I think we could easily provide a Assume we already got
If foo has changed, the transaction will fail. |
I was thinking why do people say it's "compare and swap" and not "compare and set". "Check and set" seems right. Updated the title. thanks. |
My initial thought was we can compare the string/numeric value directly without maintaining any additional property. Hence, suggested for those two data types. I think if we are aligned we want to build this functionality in some form or other, we can go down the RFC route. If I'm reading the room right, seems like we would like to build it. |
BTW @zuiderkwast ETCD provides 1/2 and from my experience, It's quite powerful. They do maintain multiple version of each key (point 1). And the API is structured as |
@hwware If I understand correctly, the client will retry if the condition doesn't match. |
Yeah I think this approach achieves an effective balance between storage efficiency and flexibility, as it could extend universally to all key types and commands. Standardizing on the |
Compare and set is a very common requirement, and I'm very happy to accept this proposal. Regarding the implementation, my intuition is:
|
After the discussion on #1087, I think we would like to build CAS functionality along with supporting caching of response of |
I disagree with one point, I think CAS is a pretty common scenario, we don't get enough request to build maybe because of |
Versioning also enables MVCC (maybe we can build multi threaded command execution :D). But, it brings in lot more complexity and challenges with it like tree like storage of different values for each version, compaction (trimming of older version value(s), new data persistence format, etc. So, we need to be thoughtful if we want to build it. |
I think supporting CAS is definitely needed. I agree with @soloestoy that in most cases (not sure 99%) the values are small and
I am not sure if that was the proposal, but I think that having this functionality ONLY as part of transactions would be very limiting. Transactions cannot be nested and run from scripts. I think having this as part of the SET/HSET is probably a very good idea. I am wondering though how exactly will the API look. There are several cases we MIGHT want to "compare" against:
Not sure if that does not complicates things by too much. We did such things in the sort function and maybe that is why noone uses it :) But was wondering if we feel that is required? |
@ranshid I just listed some ideas. They are not mutually exclusive. The DIGEST and CHECK idea is a way to extend this to all key types and not be tied to any specific command. I don't think it's very limiting that transactions can't be used from scripts. Scripts can just do the comparison themselves, using
Number 1 is my suggestion. The old value of the same key is all that matters in 99.93% of cases(???). We don't need to change the key specs for the SET command. That would be more complex. We should compare this conditional execution with other arguments that do conditional execution. They are not too complex and not too different:
|
This is the one that is most likely going to be used for optimistic locking, which is the core use case for CAS (AFAIK). The client will fetch the value, keep it in its memory, and then conditionally apply it back to the server. From the thread, doing I see @PingXie wants to see more consistency, but I think that slapping on digest for all datatypes feels like a mistake. If you want to do a conditional write on a stream, which can be megabytes in size, you don't want to constantly do a digest across the whole object. We also generally don't want to be doing hashing on the server side, since it's a computationally expensive operation, and I think a goal of Valkey should be consistent high performance and single slow commands cause latency spikes. I also don't see how that is fundamentally very different from using WATCH. You execute watch on a key, and if it's modified it invalidates the MULTI state. (It's worth mentioning that I think that I think we should just start with SET + IFEQ. I want us to be mindful that every time we launch a feature we are committing to maintaining it. A third option might be something like a module that implements the functionality, and we can let people opt-in to using that if they want. |
Before we just begin to work on this, I still have 3 concern:
|
I support this as well, but we should also introduce the HSET + IFEQ IMO, many cases AFAIK used the same logic on hashes as the one used on general STRINGS. BTW - what about MSET? |
There are already modules providing this functionality: TairString and TairHash.
It looks like "IFEQ" is key number n + 1. It is not possible to extend this command in this way. MSET doesn't have NX and XX, so I think it doesn't need IFEQ either.
HSET has the same problem as HSET: It's not possible to extend it because it is already variadic. We have HSETNX and we could add HSETIFEQ in the similar style, or a new HSET-extended which can allow args like NX, XX and IFEQ. @enjoy-binbin already suggested this in a Redis PR redis/redis#9058 with the syntax |
I suppose we could encourage more people to use these then.
You can always just do a multi-exec of a bunch of normal SETs.
|
Do we need to support all the data types in the first place? I feel we can start off with regular strings/numbers and go from there. |
Trying to drive consensus then @valkey-io/core-team please vote 👍 or 👎 on: Extending set to have the following additional syntax.
Where the command is executed, IFF the comparison-value equals the current value in the key the rest of the code will be executed. If the values aren't the same, it returns a new error |
SET with NX or XX returns |
Regarding the version numbering idea, I'd like to draw some references to memcached
IMO this is more efficient compared to comparing the value on every update operation (where the value can be small/large, or one of the items in a larger collection, or custom module data types that is harder to specify the value of in a command, etc). Another benefit of this approach is that it's much easier in the future to support additional data types like hash/list/set/sorted set/HLL/Modules etc. i.e.
|
Does that mean they need to be aware of usage of this feature while building Valkey binary? |
Yeah, I don't like this. I feel like one of the things that made memcached hard to use was all the static configuration. It didn't support dynamic configuration initially. |
The problem/use-case that the feature addresses
Allow Valkey user to perform conditional update to a key based on certain condition like matches old value/checksum. This should ideally be supported for string data type via SET command.
API changes in SET command:
Behavior change:
Where the command is executed, IFF the comparison-value equals the current value in the key the rest of the code will be executed. If the values aren't the same, it returns a new error -NOTEQUAL. Alternative: Return a (nil) instead. We'll decide between these two cases in the PR. #1215 (comment)
Memcached CAS:
https://docs.memcached.org/protocols/basic/#cas
Past discussions:
https://groups.google.com/g/redis-db/c/a4zK2k1Lefo
redis/redis#8361
redis/redis#12485
Alternatives considered
Use LUA scripting
The text was updated successfully, but these errors were encountered: