Comments (18)
TBH, I've always disliked ambiguous behavior. And in my understanding the control over whether multiple slots reside on the same node is unmanageable. Limiting the keys a command operates on to a single slot is a very deterministic practice, which also gives the cluster flexibility. Slots can be moved from one node to another without errors (this is under the assumption of atomic slot migration). But allowing commands to execute across slots while disallowing cross-node execution complicates things immensely, especially as slot distribution is subject to change.
I've also noticed discussions above about allowing only certain commands to execute across slots. In my view, this adds complexity to the interaction between client and server, and furthermore, it significantly increases the learning curve for users. I can easily imagine a scenario where, after successfully executing a cross-slot MGET
, a user will attempt to perform cross-slot RENAME
, COPY
, SUNIONSTORE
, etc. This can be particularly problematic when database administrators and application developers are from separate teams, it's a recipe for operational disaster.
Additionally, there's the question of how to handle persistence and replication for these cross-slot commands. As far as I know, many users rely on AOF for data migration. If a cross-slot SUNIONSTORE
command appears in the AOF file, it is highly likely to cause migration failure because the slots in the target cluster may well be distributed across different nodes.
There's also the planned atomic slot migration, we've previously discussed reusing the replication mechanism from primary-replica setups. If the incremental replication stream contains cross-slot commands during slot migration, this also could cause the migration to fail.
These issues I've outlined are just some of the problems I've encountered. I can imagine there would be much corner cases that cross-slot functionality might generate.
from valkey.
I kind of like this idea, but my thought has been it should be scoped down to "very" specific commands. Specifically, I think it should be the commands with logical fan-in and fan out behavior, like MGET
, MSET
, DEL
, etc. Esoteric commands like like SUNIONSTORE
probably don't need to support multi-slot.
With enable-cross-slot set to yes, Redis will ensure atomic execution of multi-key commands in individual shards, enhancing the reliability and consistency of operations.
I've mentioned this on other threads, but I don't think this should be a server feature, but instead a client capability they should be able to opt-in to.
from valkey.
Great, so can we agree on this↓?
- Client opt-in (not a config).
- Allow for read and write commands, but forbid for read-modify-write commands.
- We need to add a command flag for the read-modify-write commands, or flag them as read+write, because now they're just flagged as write. (The
*STORE
commands and the commands with aSTORE
orSTOREDIST
argument:SORT
,GEORADIUS
,GEORADIUSBYMEMBER
)
from valkey.
@bsmanit This is going to get implemented in this PR:
#707
If it does not address your concerns please let me know in the PR so I can adjust it.
from valkey.
@madolson Yes, I agree this should be very command-specific. The client should handle fan-in/fan-out behavior. Some clients, like Lettuce, already do this.
For example, Lettuce groups keys by hash slot for MGET and then sends them to the Redis server. With the new feature, it can group keys by the hash slot range owned by each shard instead. This will improve read throughput significantly.
from valkey.
Generic clients need to support both values this config. Do you think they should call CONFIG GET to find out if they can use crosslot commands or not?
I think it's better that it's opt-in for the client. Then only the clients that ask for it will get it, without affecting other clients and users. Idea:
CLIENT CROSSSLOT ON
It can be sent in pipeline with HELLO or AUTH just after connecting.
from valkey.
Btw, are you sure that multiple GET in a pipeline is much slower than MGET? I doubt it.
from valkey.
Third comment: I don't think it should be limited to just a few commands. That would just be another confusion. If it's enabled, it can apply to sunionstore and even to transactions affecting multiple slots.
from valkey.
Third comment: I don't think it should be limited to just a few commands. That would just be another confusion. If it's enabled, it can apply to sunionstore and even to transactions affecting multiple slots.
The original point of the cross-slot was to detect misuse. I think SUNIONSTORE
where the source and target are different slots is almost always misuse. The only material case I can think of is you have an intermediary storage on the same node where you are trying to "aggregate data", and then plan on deleting it. I'm not sure that is materially that important or useful.
from valkey.
The original point of the cross-slot was to detect misuse
Yeah, so that's why it should be forbidden by default. MSET is an abuse too because it appears to the end user to be atomic, which it isn't (if the client silently splits the command and sends to different nodes).
If you get some keys from SCAN, you know they're on the same node. You may want to compute an SUNION on these keys. The alternative is to compute the union on the client side, something that can always be done even for a cross-node union, so allowing crosslot is an optimization for clients in this case too.
If we want to prevent this (because we say it's an invalid use case) we'd need additional command flags and logic just to keep track of whether this is allowed for each command. I don't like it.
from valkey.
To your point about atomicity. SUNIONSTORE should probably always be an atomic operation. I can see a world where MSET is not expected to be atomic though, because you are overwriting different keys with some value. I think the main distinction that I am seeing is that there are:
- Read commands
- Write commands
- Read + modify + write commands.
That last category is the one that I don't think should be allowed to fanout and operate across slots.
from valkey.
OK so not SUNIONSTORE, but for SUNION it would be OK since it's a read command? Makes some sense now.
from valkey.
Yeah, sunion seems like it would be okay to me.
from valkey.
Yeah, I'm aligned with what you said.
We do theoretically have all this information already in the .json files, as they include r/w information for which keys. It's just a matter of computing it.
from valkey.
I see the value in cross-slot operations within a single-shard setup, and it seems to provide a seamless experience for the client application.
However, I'm unclear on how cross-slot operations would work intuitively in a multi-shard setup, assuming we are not talking about fanning out to other shards on the caller's behalf, i.e., the in-proc proxy? While the client application is aware of slot mapping, it typically doesn't have control over it. The admin and user roles are usually separate, even in self-managed environments. This separation is important because the primary reason an application desires cross-slot migration is atomicity. Simply supporting cross-slot operations isn't enough; the slots relevant to the application also need to be guaranteed on the same shard.
Additionally, I'm not convinced by the performance argument. Wouldn't pipelining already achieve the desired performance improvements?
from valkey.
Additionally, I'm not convinced by the performance argument. Wouldn't pipelining already achieve the desired performance improvements?
Performance is something we can test for sure. There is a fair amount of overhead in handling the call. It also removes one edge case from clients, which is partial success within a shard. It also makes idempotency a bit easier, since either all the operations succeeds on a shard or none of them did.
I agree with one other point though, is that this becomes a lot more compelling as a proxy feature as compared to a server feature. Maybe we should create an issue for maintaining a first party Valkey proxy?
from valkey.
this becomes a lot more compelling as a proxy feature as compared to a server feature
I'm skeptical to a proxy that tries to hide the fact that there's a cluster behind it. A proxy can't do anything more than what a client can do.
What a client or proxy would need for that is cross-node transactions. (If we ever want that, we could add 2PC with key locking, like MULTI-PREPARE-EXEC.)
from valkey.
I'm skeptical to a proxy that tries to hide the fact that there's a cluster behind it. A proxy can't do anything more than what a client can do.
One point that has come up fairly frequently is that not everyone has the ability to upgrade the client, but if you put a proxy you can independently run and update the proxy.
from valkey.
Related Issues (20)
- [NEW] Module support for Sentinels HOT 4
- [NEW][Feature Request] TTL for keys within the HASH data type HOT 2
- [BUG] Lua libs and functions aren't replicated on a cluster HOT 2
- [NEW] Output logs as JSON HOT 4
- Follow up on the capability flag during cluster meet
- [NEW] sentinel shouldn't pick slaves that are not synced with master #13533
- Stale `PONG` message causes incorrect `replicaof` updates leading to `replicaof` loops
- [Test issue] The tests don't correctly reflect the number of tests left HOT 4
- [BUG] Using more sentinels than io-threads causes high idle CPU usage on leader HOT 2
- [Test failure] Slow runners show issue with hostnames HOT 6
- [BUG] FUNCTION LOAD replicates to replica, while SCRIPT LOAD does not. HOT 1
- [Test failure] Migrated replica reports zero repl offset and rank HOT 2
- [Test failure] Instance #0, #1, #2 gets converted into a slaves HOT 1
- [Test failure] valkey-cli make source node ignores NOREPLICAS error when doing the last CLUSTER SETSLOT HOT 4
- Add [Dual Channel] Tag for Dual Channel Server Logs HOT 1
- [NEW] New data structure for TTL HOT 2
- Implement new hash table
- Embed key and TTL in robj HOT 5
- [CRASH] Assertion failed: pthread_mutex_unlock HOT 9
- [BUG] slot stuck in importing state on replica after scale-up and rebalance HOT 14
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from valkey.