Redis Cluster with Pipeline

[HeartSaVioR]

Hello, community!

I'm participating Jedis, Java Redis Client, and trying to achieve Redis Cluster with Pipeline.

Jedis community is discussing about it, https://groups.google.com/d/msg/jedis_redis/u6j8slokO3E/Dh5Q94TRjJUJ .

But it stucks because of concerning race-condition and its complexity.

I'm trying to make feature simple and easy by adding restrictions, all commands in one pipeline should point to same slot.

(It's for minimizing target node.)

But issue still exists because of slot migration.

AFAIK, there're two states for slot migration that some keys already migrated to other node, or some keys are not migrated yet.

Slot migration doesn't block commands so we can't think about single-thread simplicity from Redis Cluster.

We can't safe with checking slot migration before sending command, because Redis Cluster can migrate slot between received slot state and sending command.

Even though Redis Cluster can migrate slot between pipelined commands.

  

So I'm thinking about adding new restrictions, all commands in one pipeline doesn't guarantee to be executed sequentially.

With restrictions we can run pipelined commands, and build responses, and filter those which need to retry, and run again.

But I wonder it is best approach we can choose, so I'm happy with all your reviews (since I may be wrong) and any ideas about this.

Thanks in advance!

Regards.

Jungtaek Lim (HeartSaVioR)

to. antirez and mattsta

Do you have a plan to support Cluster Pipeline by introducing some mechanisms or commands?

[Josiah Carlson]

As I mentioned in another thread [1], it is effectively trivial to come up with a case where executing commands in a different order from their original intent can lead to data inconsistency. Or in this case, incorrect data.

Your best bet (in my opinion, also mentioned in [2]) is to require that all pipelines be transactional via WATCH [all keys being operated on], MULTI, [all of your commands], EXEC. Or push users to use Lua scripts, providing all KEYS that are used as part of the call. Either way, you should be able to quickly discover if the transaction/pipeline could complete.

 - Josiah

[1] https://groups.google.com/d/msg/redis-db/ho0OD7H8VvE/T7fWQ_nEVoYJ

[2] https://groups.google.com/d/msg/redis-db/38Oml2I5pmw/4ExS-qn_T3AJ

--
You received this message because you are subscribed to the Google Groups "Redis DB" group.
To unsubscribe from this group and stop receiving emails from it, send an email to redis-db+u...@googlegroups.com.
To post to this group, send email to redi...@googlegroups.com.
Visit this group at http://groups.google.com/group/redis-db.
For more options, visit https://groups.google.com/d/optout.

[HeartSaVioR]

Thanks Josiah for feedback!

I've thought that my idea doesn't look good, but in client library's point of view I had no ideas other than these. :)

Pipeline should let us reach higher throughput so I wish to add it.

Surely I should document that users about to use pipeline with Redis Cluster should take care of those restrictions if we REALLY think it's OK to apply idea.

Your ideas completely make sense, but I afraid it can be done by client library side with not affecting user's intention.

We could discuss WATCH - MULTI - EXEC pattern though seems like it could be complex.

Thanks again!

ps. Actually I've implemented supporting Async (https://github.com/xetorthio/jedis/pull/713), but cannot apply it to Redis Cluster because of handling MOVED, ASK. I'm finding a way to handle this.

2014년 11월 15일 토요일 오전 8시 46분 15초 UTC+9, Josiah Carlson 님의 말:

[Josiah Carlson]

On Fri, Nov 14, 2014 at 4:13 PM, HeartSaVioR <kab...@gmail.com> wrote:

Thanks Josiah for feedback!

I've thought that my idea doesn't look good, but in client library's point of view I had no ideas other than these. :)

Pipeline should let us reach higher throughput so I wish to add it.

Surely I should document that users about to use pipeline with Redis Cluster should take care of those restrictions if we REALLY think it's OK to apply idea.

The problem is that while you may want an async pipeline for better performance, you can't guarantee that the commands being executed won't destroy your data if performed in the wrong order. And if you can't guarantee that your data will be fine, you have to document with: WARNING: using async behavior on Redis cluster could cause incorrect execution and may destroy some of your data. But because most users won't read the documentation, when someone inevitably destroys their data, it will be *your* fault because you released a feature with known potential to destroy data.

At present, I do not believe that there is a method by which you can get asynchronous pipelines between your client and Redis cluster to guarantee sequential execution. If Redis cluster published events for key migrations (migration start, migration finish) in addition to shard migrations (which don't seem to exist, but which should at least have: shard starting migration from X to Y, shard finished migration from X to Y), you could substantially limit the potential exposure for data loss. The exposure is still nonzero, but it is at least limited to network latency and/or outgoing buffers (both of which I still think are scary).

Really though, this should be addressed on the Redis cluster side of things. There is a method that would guarantee correct execution, and would let async execution work correctly in the vast majority of situations. This first pass at a solution (as I describe it below) requires 1 new CLIENT sub-command with 1 argument that could be one of 3 values. You could probably cut it down to 2 different values ('yes' and 'no'), but I'm a fan of the Python zen, "Explicit is better than implicit".

1. When a client connects and wants to send commands to Redis asynchronously, they call "CLIENT ASYNC yes" (until this is called, the below behaviors do not apply)

2. If while executing commands, the server would return a MOVED or ASK response, the server marks the connection as 'always ask', which means that all commands executed from then return the ASK response

3. To recover a connection from being in 'always ask' mode, the client would execute "CLIENT ASYNC reset", which puts the connection back into normal async mode as it was at the end of #1

Assuming that your client didn't discard the commands that were sent (but failed due to MOVED/ASK), you can execute "CLIENT ASYNC reset", handle the re-execution of commands as you would if it wasn't async.

To disable async client processing, which also clears the 'always ask' status, you would call "CLIENT ASYNC no".

For many users, having an additional configuration option for "clients-default-to-async" that defaults to "no" might make sense, but I wouldn't suggest adding it simply because any client that wants async behavior can trivially add this one command as part of connection creation. Clients would perform this in the exact same way that they already use AUTH on connection creation.

Now we just need to convince Salvatore that all async clients are currently broken in Redis Cluster and that this would solve the problem.

Your ideas completely make sense, but I afraid it can be done by client library side with not affecting user's intention.

We could discuss WATCH - MULTI - EXEC pattern though seems like it could be complex.

Doing WATCH/MULTI/EXEC can really only work if you batch commands together, waiting for the last group to finish executing before sending this group. It is definitely not ideal.

Thanks again!

ps. Actually I've implemented supporting Async (https://github.com/xetorthio/jedis/pull/713), but cannot apply it to Redis Cluster because of handling MOVED, ASK. I'm finding a way to handle this.

In regular Redis, async execution is perfectly fine because keys don't move :)

 - Josiah

[HeartSaVioR]

2014년 11월 15일 토요일 오전 10시 51분 22초 UTC+9, Josiah Carlson 님의 말:

On Fri, Nov 14, 2014 at 4:13 PM, HeartSaVioR <kab...@gmail.com> wrote:

Thanks Josiah for feedback!

I've thought that my idea doesn't look good, but in client library's point of view I had no ideas other than these. :)

Pipeline should let us reach higher throughput so I wish to add it.

Surely I should document that users about to use pipeline with Redis Cluster should take care of those restrictions if we REALLY think it's OK to apply idea.

The problem is that while you may want an async pipeline for better performance, you can't guarantee that the commands being executed won't destroy your data if performed in the wrong order. And if you can't guarantee that your data will be fine, you have to document with: WARNING: using async behavior on Redis cluster could cause incorrect execution and may destroy some of your data. But because most users won't read the documentation, when someone inevitably destroys their data, it will be *your* fault because you released a feature with known potential to destroy data.

Yeah, I didn't think about it, and your thought makes sense. It can be blamed even though we've documented it to "WARNING: at your own risk". If then it's better to drop feature about it.

 

At present, I do not believe that there is a method by which you can get asynchronous pipelines between your client and Redis cluster to guarantee sequential execution. If Redis cluster published events for key migrations (migration start, migration finish) in addition to shard migrations (which don't seem to exist, but which should at least have: shard starting migration from X to Y, shard finished migration from X to Y), you could substantially limit the potential exposure for data loss. The exposure is still nonzero, but it is at least limited to network latency and/or outgoing buffers (both of which I still think are scary).

Yes, actually we can ask Redis Cluster that slot is migrating, but as I stated first, there're no barrier to prevent slot migration before running pipeline commands even though Redis Cluster tells me this slot is not migrating now. 

So it (including handling event you've stated) is useless.

 

Really though, this should be addressed on the Redis cluster side of things. There is a method that would guarantee correct execution, and would let async execution work correctly in the vast majority of situations. This first pass at a solution (as I describe it below) requires 1 new CLIENT sub-command with 1 argument that could be one of 3 values. You could probably cut it down to 2 different values ('yes' and 'no'), but I'm a fan of the Python zen, "Explicit is better than implicit".

1. When a client connects and wants to send commands to Redis asynchronously, they call "CLIENT ASYNC yes" (until this is called, the below behaviors do not apply)

2. If while executing commands, the server would return a MOVED or ASK response, the server marks the connection as 'always ask', which means that all commands executed from then return the ASK response

3. To recover a connection from being in 'always ask' mode, the client would execute "CLIENT ASYNC reset", which puts the connection back into normal async mode as it was at the end of #1

Assuming that your client didn't discard the commands that were sent (but failed due to MOVED/ASK), you can execute "CLIENT ASYNC reset", handle the re-execution of commands as you would if it wasn't async.

To disable async client processing, which also clears the 'always ask' status, you would call "CLIENT ASYNC no".

For many users, having an additional configuration option for "clients-default-to-async" that defaults to "no" might make sense, but I wouldn't suggest adding it simply because any client that wants async behavior can trivially add this one command as part of connection creation. Clients would perform this in the exact same way that they already use AUTH on connection creation.

Now we just need to convince Salvatore that all async clients are currently broken in Redis Cluster and that this would solve the problem.

Actually I've thought other approach, like RWLock on slot migrate and pipeline command.

1. If client sends batch commands, it checks that all commands are targeted to one slot.

2. If then, it checks that slot is being migrated.

    2.a. If slot is migrating, Redis responses "cannot execute batch to slot".

3. If slot is not migrating, Redis executes batch commands with SLOT WRITE LOCK.

    3.a. When SLOT WRITE LOCK is on, request to slot migration would wait for LOCK.

5. Redis turns off SLOT WRITE LOCK if batch command is completed.

Maybe waiting queue could be fair queue to prevent slot migrate request to wait forever. 

Your ideas completely make sense, but I afraid it can be done by client library side with not affecting user's intention.

We could discuss WATCH - MULTI - EXEC pattern though seems like it could be complex.

Doing WATCH/MULTI/EXEC can really only work if you batch commands together, waiting for the last group to finish executing before sending this group. It is definitely not ideal.

Pipeline is based on batch, so it should make sense.

[Josiah Carlson]

Using WATCH/MULTI/EXEC offers almost the exact same functionality (reject the group of operations before execution if the shard is moving) without the additional commands.

Your ideas completely make sense, but I afraid it can be done by client library side with not affecting user's intention.

We could discuss WATCH - MULTI - EXEC pattern though seems like it could be complex.

Doing WATCH/MULTI/EXEC can really only work if you batch commands together, waiting for the last group to finish executing before sending this group. It is definitely not ideal.

Pipeline is based on batch, so it should make sense.

In your first message in the thread, you linked to the Jedis mailing list where you mentioned that this was async. But now you say that it is batched. If it's batched, then every pipeline is a transactional pipeline, and both variants should just be the same.

I just checked the source code of MULTI/EXEC transactions in Redis cluster, and the WATCH part is not necessary to validate that keys are fine. Before executing any commands in the transaction, Redis checks that all keys are in the same slot, that the slot is on the server, and that all of the keys are available.

Cluster may still be a bit broken in an async scenario, I'll add a bug to describe the issue.

 - Josiah

[임정택]

Sorry Josiah, it's not async, but I've confused pipeline and batch.
Yes, it should be batch, not pipeline.

Btw, some commands inside of MULTI fails because of MOVED, it doesn't rollback successful commands, right?
(Redis discards whole commands if it has ERROR on commands. I'm curious what Redis actually treat with MOVED and ASK.)
If it is, it's same as I was sketched.

If it's not working...
We really need a barrier which checks those keys are not moving now, and prevents keys movement during running batched commands.
And only server side (Redis) can do it.

Please correct me if I'm wrong.

Best.
Jungtaek Lim (HeartSaVioR)

To unsubscribe from this group and stop receiving emails from it, send an email to redis-db+unsubscribe@googlegroups.com.

To post to this group, send email to redi...@googlegroups.com.
Visit this group at http://groups.google.com/group/redis-db.
For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to a topic in the Google Groups "Redis DB" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/redis-db/4I0ELYnf3bk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to redis-db+unsubscribe@googlegroups.com.

[Josiah Carlson]

On Wed, Nov 19, 2014 at 2:14 PM, 임정택 <kab...@gmail.com> wrote:

Sorry Josiah, it's not async, but I've confused pipeline and batch.
Yes, it should be batch, not pipeline.

Btw, some commands inside of MULTI fails because of MOVED, it doesn't rollback successful commands, right?

When Redis receives EXEC, it:

1. looks at all of the commands that were sent after the most recent MULTI

2. extracts the keys from those commands

3. verifies that all of the keys are in the same slot

4. verifies that the slot is in this machine

5. verifies that all of the keys are available in this machine (which handles in-progress slot migration)

No execution happens inside a MULTI/EXEC block unless Redis cluster is pretty sure that it will complete successfully. Which means that you shouldn't get an error at all.

To repeat again: wrap your batch of commands in MULTI/EXEC. While that won't prevent slot migration, no migration happens during the MULTI/EXEC execution, and all keys are checked before execution of any commands inside the MULTI/EXEC.

 - Josiah

[HeartSaVioR]

Thanks Josiah! 

I've read Redis source code now, and you're right.

Actually I didn't think slot migrate should be performed by sending MIGRATE to all keys manually.

But I see redis-trib.rb now, and it's all covered from CLIENT side.

Moving keys is not running async, it's exactly same thing to normal command.

I was mistaken whole things due to misunderstand this.

Based on it, we can stick with MULTI-EXEC pattern. 

Btw, I found another undocumented command "keysinslot" from redis-trib, which may helps some scenarios. :)

Sorry for taking your time, and thanks again!

Regards.

Jungtaek Lim (HeartSaVioR)

2014년 11월 20일 목요일 오전 8시 27분 26초 UTC+9, Josiah Carlson 님의 말:

You received this message because you are subscribed to a topic in the Google Groups "Redis DB" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/redis-db/4I0ELYnf3bk/unsubscribe.

To unsubscribe from this group and all its topics, send an email to redis-db+u...@googlegroups.com.

To post to this group, send email to redi...@googlegroups.com.
Visit this group at http://groups.google.com/group/redis-db.
For more options, visit https://groups.google.com/d/optout.

[mrazola]

Hi!

Any news regarding this feature request? It would be very interesting to have JedisCluster suport transactions (if keys belong to the same node).

[Guillermo Casanova]

I would love to see this implemented, did this get any further? Regards!

[Josiah Carlson]

You may get better results asking on the Jedis mailing list.

 - Josiah