Redis scripting documentation draft

[Salvatore Sanfilippo]

Hello!

Finally we have some doc on the current implementation of the
scripting feature in Redis:

http://redis.io/commands/eval

Please help me improving this documentation.

Also I would like to use this thread to discuss the Redis scripting
feature semantics and implementation details in client libraries.
Yesterday I saw a few comments on twitter and tried to reply, but now
with the actual document published and more than 140 characters I bet
it's a better environment for discussions ;)

Ciao,
Salvatore

--
Salvatore 'antirez' Sanfilippo
open source developer - VMware

http://invece.org
"We are what we repeatedly do. Excellence, therefore, is not an act,
but a habit." -- Aristotele

[Sergei Tulentsev]

typo:
> in order to enstablish

ambiguity:
> The only way to flush the script cache is by explicitly calling the EVALCTL FLUSH
> SCRIPT FLUSH. This command is the only way to force Redis to flush the scripts cache.

That's all I got right now :-)

--
You received this message because you are subscribed to the Google Groups "Redis DB" group.
To post to this group, send email to redi...@googlegroups.com.
To unsubscribe from this group, send email to redis-db+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/redis-db?hl=en.

--
Best regards,
Sergei Tulentsev

[Salvatore Sanfilippo]

Thank you, fixed.

[Louis-Philippe Perron]

Salvatore,

It's probably already part of your implementation, but have you though of having SCRIPT LOAD return the corresponding SHA of the script?

L-P

[Salvatore Sanfilippo]

On Tue, Oct 25, 2011 at 2:42 PM, Louis-Philippe Perron
<lppe...@gmail.com> wrote:
> Salvatore,
> It's probably already part of your implementation, but have you though of
> having SCRIPT LOAD return the corresponding SHA of the script?

Good idea, sorry, fixing.

[Louis-Philippe Perron]

also,

TYPO:

..." in an atomic way: nor other scripts or other Redis commands"...

[Salvatore Sanfilippo]

Fixed now, thanks.

[Salvatore Sanfilippo]

Fixed as well, thanks a lot.

[catwell]

I saw a few typos when reading but didn't stop to correct them.
I'll make another pass later.

I just wanted to discuss that for now:

> On the other hand defining commands using a special command or via redis.conf would be a problem for a few reasons:

> * Different instances may have different versions of a command implementation.
> * Deployment is hard if there is to make sure all the instances contain a given command, especially in a distributed environment.
> * Reading an application code the full semantic could not be clear since the app would call commands defined server side.

> In order to avoid the above three problems and at the same time don't incur in the bandwidth penalty Redis implements the EVALSHA command.

I still think that defining commands with a given name instead of
using SHAs can be interesting for some use cases, and that SCRIPT LOAD
should work that way. Why?

* You can still compute the SHA on the client side and obtain the same
result if you want.

* You can still version the command manually if needed (eg. MYCMD ->
MYCMD2...). OTOH you can also *not* do it and hot-patch a script if
you find a bug / need to fix its behavior.

* This makes it possible to separate applications that load scripts
from applications that use scripts. For instance there could be a
script in redis-extend that loads a command into a Redis server.

* I don't think there's a semantic problem: if you use scripts this
way you will document them exactly in the same way that you would
document Redis commands.

* I don't think this makes deployment harder. I think it makes it
simpler because it separates it from the application. If you can
deploy a Redis cluster then you can probably deploys scripts too. OTOH
if you're like me you probably have more applicative servers than
Redis servers, and you would currently have to modify them all if you
wanted to update a script.

The difference between both approaches lies IMO in whether you
consider scripts as one-shot tools or real new commands, whether
they're the job of dev or ops.

Now I can live with the design proposed by Antirez, especially if
SCRIPT LOAD returns the SHA of the script (because at least I will not
have to make my client app depend on a crypto lib...).

(ugly hack)
I do see a way to decouple like I wish it were with the current design
but it's convoluted...

OPS
---

$mysha = SCRIPT LOAD ...
HSET script_shas my_cmd $mysha

CLIENTS
-------

On app load: $my_cmd_sha = HGET script_shas my_cmd
If evalsha $$my_cmd_sha fails: same thing.

(/ugly hack)

OK, that was food for thought, now discuss :)

[catwell]

Another note: I hadn't noticed but there is no "SCRIPT UNLOAD"
command. This at least would be needed to signify applications that
they cannot use a script anymore...

[catwell]

Typos etc I saw (certainly not exhaustive):

< The following example can clarify what stated above
> The following example can clarify what is stated above

< Lua arrays are returend
> Lua arrays are returned

< It is possible to call Redis program from a Lua script
> It is possible to call a Redis command from a Lua script

< if a Redis command call will result into an error
> if a Redis command call causes an error

< will make EVAL to fail
> will make EVAL fail

< in order to enstablish what are the keys the command will operate on
> in order to establish what the keys the command will operate on are

< In order for this to be true for EVAL also keys must be explicit
> In order for this to be true for EVAL too, keys must be explicit

< the scripting feature was designed in order to play well with it
> the scripting feature was designed to play well with it

< is designed in a way that if a Redis type
> is designed in such a way that if a Redis type

> the result is the same as of the initial value
< the result is the same as the initial value

> the script is not knonw
< the script is not known

> if the client is taking the master very busy sending scripts
< if the client is making the master very busy by sending scripts

> turing this scripts
< turning these scripts

> using Lua random number generator
< using the Lua random number generator

> will result in a different random elements
< will result in different random elements

> All the other librareis are standard
< All the other libraries are standard

> Scripts also are subject to a maxium execution time
< Scripts also are subject to a maximum execution time

> the order of executin of commands
< the order of execution of commands

> add SCRIPT LOAD commands on top of the pipeline as requierd
< add SCRIPT LOAD commands on top of the pipeline as required

[Salvatore Sanfilippo]

On Tue, Oct 25, 2011 at 3:18 PM, catwell <catwell...@catwell.info> wrote:

> I still think that defining commands with a given name instead of
> using SHAs can be interesting for some use cases, and that SCRIPT LOAD
> should work that way. Why?
>
> * You can still compute the SHA on the client side and obtain the same
> result if you want.
>
> * You can still version the command manually if needed (eg. MYCMD ->
> MYCMD2...). OTOH you can also *not* do it and hot-patch a script if
> you find a bug / need to fix its behavior.
>
> * This makes it possible to separate applications that load scripts
> from applications that use scripts. For instance there could be a
> script in redis-extend that loads a command into a Redis server.
>
> * I don't think there's a semantic problem: if you use scripts this
> way you will document them exactly in the same way that you would
> document Redis commands.
>
> * I don't think this makes deployment harder. I think it makes it
> simpler because it separates it from the application. If you can
> deploy a Redis cluster then you can probably deploys scripts too. OTOH
> if you're like me you probably have more applicative servers than
> Redis servers, and you would currently have to modify them all if you
> wanted to update a script.
>
> The difference between both approaches lies IMO in whether you
> consider scripts as one-shot tools or real new commands, whether
> they're the job of dev or ops.

Hi, thanks for the feedback!

I think there is some fundamental error in your reasoning, as already
stated on twitter ;)
And this error is in mixing the way applications will end scripts to
the server, and the semantics and API from the point of view of the
client.

From the point of view of a client lib there are many ways to expose scripting.

One client lib will just expose EVAL and EVALSHA directly.
Another client lib instead will use an API like the following, perhaps:

redis.register_script("shuffte-list", SHUFFLE_LIST_LUA_CODE,1,0); //
Note: final 1,0 means for instance: expect 1 key, 0 arguments.
redis.shuffle_list("mylist");

This is a way, others will just prefer a raw approach were EVAL is a
wrapper for sending EVALSHA, then retrying with EVAL if NOSCRIPT is
returned. And so forth.

So clients can pick different designs about it, completely legit.

Then there is the other side. Sending scripts to the server. Whatever
you do on the client, why on the earth should you fight with the idea
that a Redis server needs to be instructed with commands after it is
restarted or upgraded? Anyway you don't want the idea of an external
program managing your script collection, or registering scripts server
side in a way that you can define/delete them.
This will just make your operations more complex, without a single gain.

SCRIPT LOAD is only a way to ensure a given script exists, and this is
useful for pipelining. You can also use it if your API does something
like redis.register_script() since it makes sense.
But when you call your command you should ALWAYS have a body that is
associated with a command, compute the SHA1 (you can of course cache
it in your Redis object) and send EVALSHA, and if it fails, redefine
it.

Stopping and Restarting Redis. Or even using a different instance
while the server is running (imagine Redis Cluster) should never pose
problems about what scripts are defined and what not.

So, every client that will not:

1) For normal requests: use EVALSHA and later EVAL if needed.
2) For pipelined requests: use SCRIPT EXISTS ..., and pipeline SCRIPT
LOAD commands before the actual pipeline, and send just EVALSHA in the
pipeline.

is from my point of view broken.

Note that the Redis script cache has a very interesting semantic of
not forgetting scripts, and SCRIPT FLUSH should only be called when a
Redis instance is emptied in a cluster environment to provide it to
another user, or in general when You Know What You Are Doing, so... if
you are taking a permanent connection to Redis in your client library
like redis-rb does, you can even remember that you did SCRIPT LOAD for
a given script already, so you can just go for EVALSHA in a pipeline
if all your scripts are already defined. Just: make sure to flush your
scripts client-side cache if you reconnect to Redis after a network
error.

Cheers,

[Salvatore Sanfilippo]

This IMHO not only does not make sense as a script that does no longer
exists from the point of view of an application should never get its
SHA called, but is remarkably dangerous as it violates our script
cache contracts of not deleting scripts.

Salvatore

> --
> You received this message because you are subscribed to the Google Groups "Redis DB" group.
> To post to this group, send email to redi...@googlegroups.com.
> To unsubscribe from this group, send email to redis-db+u...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/redis-db?hl=en.
>
>

--

[Salvatore Sanfilippo]

Thank you

> --
> You received this message because you are subscribed to the Google Groups "Redis DB" group.
> To post to this group, send email to redi...@googlegroups.com.
> To unsubscribe from this group, send email to redis-db+u...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/redis-db?hl=en.
>
>

--

[catwell]

OK, I understand your point of view.

I find it clearer to separate scripts from the application
but something like your redis.register_script will be enough
for me, and I agree that using SHAs only is better in the
context of Redis Cluster.

I do want to be able to call scripts like normal Redis
commands on the client side so I will probably write
code that allows me to do that (using EVALSHA with a
fallback to EVAL). This will make all the client apps
require the source code of the scripts and ability to
compute SHAs though.

[Salvatore Sanfilippo]

On Tue, Oct 25, 2011 at 4:09 PM, catwell <catwell...@catwell.info> wrote:

> I do want to be able to call scripts like normal Redis
> commands on the client side so I will probably write
> code that allows me to do that (using EVALSHA with a
> fallback to EVAL). This will make all the client apps
> require the source code of the scripts and ability to
> compute SHAs though.

As I said this is very legit, as it is an API stuff so arguments are
about aesthetic not functionality.

Even separating Lua and Ruby/Python/Perl/Whatever code this way what
is good is that Lua scripts are *still* part of the applications,
inside some scripts.rb file or alike, so that the full semantics of
the application is evincible reading the full source code.

[catwell]

> Even separating Lua and Ruby/Python/Perl/Whatever code this way what
> is good is that Lua scripts are *still* part of the applications,
> inside some scripts.rb file or alike, so that the full semantics of
> the application is evincible reading the full source code.

This is the only thing we don't agree on I guess :) You consider
scripts to be part of the application whereas I consider general use
scripts like those in redis-extend to be part of the server.

This is why including their source code in the application is strange
to me, exactly in the same way that including Redis' source code
would be.

[catwell]

On Oct 25, 5:00 pm, catwell <catwell-goo...@catwell.info> wrote:

> This is the only thing we don't agree on I guess :) You consider
> scripts to be part of the application whereas I consider general use
> scripts like those in redis-extend to be part of the server.
>
> This is why including their source code in the application is strange
> to me, exactly in the same way that including Redis' source code
> would be.

To use a RoR analogy (sorry for those who don't get it): including
scripts in the application sounds like Rails Plugins, and I want
gems (ie. system-wide settings).

[Seppo]

Hello,
I just finished reading the documentation and have some notes/questions
regarding it or the functionality.

Isn't inconsistent to use "ok" as field for all status?

Lua number -> Redis integer reply
Clarification on non integers numbers. By my tests math.floor (or
truncation) is being applied:
redis 127.0.0.1:6379> EVAL "return 1.9" 0
(integer) 1

Should add a note about multi level recursive multi-bulk being added?
Previous version of Redis clients may not support it

I don't know if it's a bug or not, but lua's tables not starting on 1
are not handled as I would expect it. E.g.:
redis 127.0.0.1:6379> EVAL "t = {}; t[1] = 3; return t;" 0
1) (integer) 3
redis 127.0.0.1:6379> EVAL "t = {}; t[2] = 3; return t;" 0
(empty list or set)

> Scripts also are subject to a maxium execution time of five seconds.
This default timeout is huge since a script should run usually in a sub
millisecond amount of time. The limit is mostly needed in order to avoid
problems when developing scripts that may loop forever for a programming
error.
What happens if the master executes a script and a slave hits timeout?

Cheers,
Seppo

[Marc Byrd]

I agree that the sha1 of the script should be returned when it's run.

Further, it seems powerful to me to put scripts, which are strings, into existing data structures in redis rather than invent new ones (do not repeat - DRY) - , e.g. 

hset redis-internal-reserved::  scriptSHA1 script

Further it would be nice to be able to set expiration on a script that is thus hset.

This would allow developers to get list of cached scripts, etc.  

Also, it seems to me the script_id (or script key or sha1 hash) should not include specific argument values, just calling pattern.  Using the same script on the same data repeatedly seems like a rare use case compared to the generalization.  

We could also think about allow users to name their scripts instead of the system (or in addition to).  Within this script name, they can namespace as they like to indicate their internal version number, etc. 

I recognize that a given client lib could imitate some of these capabilities.  However, consider the costs of doing this for every language binding, inconsistencies between each, and the clarity lost for direct mapping between redis.io command reference and, e.g., ezmobius' rubygem command reference.  

m

[Pieter Noordhuis]

Hi Seppo,

Regarding the "ok" field: there is no Redis protocol equivalent for a
table with such an entry. Only tables with numeric indexes are
converted to Redis multi bulks. The float maps to an integer reply; if
you want to return the string (just as double scores for sorted sets),
you need to call "tostring" on it to make Redis convert it to a bulk.

You are correct about deeper nested multi bulks: not all clients
support them. The Ruby client (redis-rb) and the C client (hiredis),
do.

Converting a Lua table to the Redis protocol is done by iterating
starting at field 1. Anything that is not defined in between is a mark
for Redis to stop. Lua tables are key/value maps, so iterating over
numeric keys in order can only be done by starting at 1.

Preventing timeout on slaves is necessary, thanks for mentioning.

Cheers,
Pieter

[Salvatore Sanfilippo]

On Tue, Oct 25, 2011 at 7:17 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:
> Preventing timeout on slaves is necessary, thanks for mentioning.

Indeed... good point, simply if an instance is a slave it will not
care about enforcing timeouts.

Thanks,

[Pieter Noordhuis]

This is, however, something that may diverge the data sets. If a
script is terminated because of its runtime, it may already have
executed a series of commands. If the script doesn't get replicated,
these commands are not executed. I think this can only be solved by
aggregating the commands that are executed by a script on the protocol
level, and using the aggregate as fallback for replication when the
script gets terminated.

What do you think?

Cheers,
Pieter

[Salvatore Sanfilippo]

On Tue, Oct 25, 2011 at 8:05 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:
> This is, however, something that may diverge the data sets. If a
> script is terminated because of its runtime, it may already have
> executed a series of commands. If the script doesn't get replicated,
> these commands are not executed. I think this can only be solved by
> aggregating the commands that are executed by a script on the protocol
> level, and using the aggregate as fallback for replication when the
> script gets terminated.
>
> What do you think?

This is definitely an issue... actually the timeout should only happen
when things are really... serious, that is, you have a bug in your
script that is going to loop forever (or a lot more than expected),
since even the 5 seconds default are an huge value.

For replication it is not a big issue, as we can just close the
replication link when the master script timeouts. This will force a
resync.
For AOF it is more complex than that... thinking about it... Thanks
for the good feedback.

Salvatore

[Damian Janowski]

On Tue, Oct 25, 2011 at 10:46 AM, catwell <catwell...@catwell.info> wrote:
> Typos etc I saw (certainly not exhaustive):

Typos fixed.

[Salvatore Sanfilippo]

Hi again,

I slept on this, and I think we are going to be something rather extreme.

Let alone that we end with problems in the AOF/replication link, but
the biggest problem is that the scripting contract is broken if we
abort the script half-way, since the application has no way to know
what was done and what was not: in short this breaks scripts
consistency.

So this is my idea: if a script that has done no writes at all so far
goes over the setup max time we abort it.
If a script that already sent even a single write command goes after
the max time, we need to call SHUTDOWN and terminate the server.

The above solution can appear bold at first, but it is the *only* way
to preserve consistency.
Also:

1) Scripts should never run usually for a long time. This timeout
should be set to two order of magnitudes bigger value compared to what
the max script execution time should be.
2) Scripts usually run a lot of time because of bugs, for debugging it
is not going to be a big problem that the server shuts down.

I really think we don't have other options but would love to get some
feedback on that.

Salvatore

[Nathan Fritz]

Instead of replicating the script, how about just replicating the resulting actions of the script? That would simplify more than just this scenario.

-Fritzy

[Louis-Philippe Perron]

Why not set sensible defaults and allow redis.conf overrides?

like having a "SCRIPT_TIMEOUT" to set duration and

a "SCRIPT_SHUTDOWN" to set if server would shutdown on script fail.

L-P

[Andy McCurdy]

I agree with Nathan. Replicating only the actual write operations solves not only this issue, but also avoids the special case treatment of random and timestamps.

For what it's worth, this is how many other database replication systems work as well (MySQL, etc.)

[Salvatore Sanfilippo]

On Wed, Oct 26, 2011 at 5:44 PM, Nathan Fritz <natha...@gmail.com> wrote:
> Instead of replicating the script, how about just replicating the resulting
> actions of the script? That would simplify more than just this scenario.

Hi Nathan,

unfortunately things are different. Even if Redis had no replication,
nor consistency at all, a script running, doing half the work it
should, and then terminating for timeout, will leave an inconsistent
data set.

So actually the problems with AOF and replication were trying to tell
us that our semantic of interrupting scripts was broken in a much more
fundamental way.

Salvatore

[Salvatore Sanfilippo]

On Wed, Oct 26, 2011 at 5:45 PM, Louis-Philippe Perron
<lppe...@gmail.com> wrote:
> Why not set sensible defaults and allow redis.conf overrides?
> like having a "SCRIPT_TIMEOUT" to set duration and
> a "SCRIPT_SHUTDOWN" to set if server would shutdown on script fail.

Hi Louis-Philippe, I think not shutting down is not an option if the
script already did even a single write, otherwise the data set is left
into an inconsistency state.

Salvatore

[catwell]

On Oct 26, 6:08 pm, Salvatore Sanfilippo <anti...@gmail.com> wrote:

> Hi Louis-Philippe, I think not shutting down is not an option if the
> script already did even a single write, otherwise the data set is left
> into an inconsistency state.

Shutting down sounds overkill though. Especially if the server is
*not*
part of a Redis Cluster, so it should be configurable.

If the server is in a Redis cluster, would it be an option to:

* delete the keys touched by the script
* recover them from another server

?

[Pieter Noordhuis]

The root cause for potential inconsistency is terminating the script
in the first place. If Redis is going to shutdown when a script takes
too long to complete and has done a write, the timeout is only useful
for debugging where you want to be notified if it takes too long.

Every client to a Redis server has root-powers in Redis land.
Executing a FLUSHALL or a long-running script is all the same, and we
trust clients to "do the right thing". So, instead of terminating the
script, why not let it run forever when it needs to and thereby avoid
introducing consistency problems. Even with a default timeout that is
huge, CPUs will eventually get pegged, and perfectly fine scripts will
eventually hit that timeout. Having Redis shutdown in that case,
doesn't sound right to me. I think having a latency burst in such
cases is much better than shutting down the server altogether.

Again, we trust users to do the right thing, and users should know
that with scripting comes great responsibility ;-)

In short: I'm in favor of not timing out scripts.

Cheers,
Pieter

[Salvatore Sanfilippo]

On Wed, Oct 26, 2011 at 6:31 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:

> In short: I'm in favor of not timing out scripts.

+1...

I want to just add this: the trap that used to terminate the script
instead of being removed at all can be turned into:
redisLog(REDIS_WARNING, "Hey... but we wanted to trust our users :(");

Makes sense?

[Pedro Melo]

Hi,

On Wed, Oct 26, 2011 at 5:31 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:
> Again, we trust users to do the right thing, and users should know
> that with scripting comes great responsibility ;-)
>
> In short: I'm in favor of not timing out scripts.

+1. I've been reading this thread and so far this one seems to be the
saner approach. SHUTDOWN is too aggressive, recover from something
like this without MVCC is next to impossible...

If a programmer is using Redis scripting in a way that it can block
the entire server for a long time, Redis should do what the programmer
asked him (her?) to do.

Bye,
--
Pedro Melo
@pedromelo
http://www.simplicidade.org/
http://about.me/melo
xmpp:me...@simplicidade.org
mailto:me...@simplicidade.org

[Pedro Melo]

On Wed, Oct 26, 2011 at 5:32 PM, Salvatore Sanfilippo <ant...@gmail.com> wrote:
> On Wed, Oct 26, 2011 at 6:31 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:
>
>> In short: I'm in favor of not timing out scripts.
>
> +1...
>
> I want to just add this: the trap that used to terminate the script
> instead of being removed at all can be turned into:
> redisLog(REDIS_WARNING, "Hey... but we wanted to trust our users :(");

"You just shoot yourself in the foot... with style!"

> Makes sense?

IMHO, best of the current scenarios/solutions.

[catwell]

On Oct 26, 6:32 pm, Salvatore Sanfilippo <anti...@gmail.com> wrote:

> On Wed, Oct 26, 2011 at 6:31 PM, Pieter Noordhuis <pcnoordh...@gmail.com> wrote:
> > In short: I'm in favor of not timing out scripts.
>
> +1...

+1

[Louis-Philippe Perron]

>> In short: I'm in favor of not timing out scripts.

+1

Keeps it simple...  

[Thomas Fritz]

Maybe i am overseeing something, but why not encapsulate every script
in a transaction, and if the script is timing out throws any error,
redis performs a rollback?
Does this solve anything?

kind regards

---
Thomas FRITZ
web http://fritzthomas.com
twitter http://twitter.com/thomasf

2011/10/26 catwell <catwell...@catwell.info>:

[Pedro Melo]

Hi,

On Thu, Oct 27, 2011 at 12:09 PM, Thomas Fritz <frit...@gmail.com> wrote:
> Maybe i am overseeing something, but why not encapsulate every script
> in a transaction, and if the script is timing out throws any error,
> redis performs a rollback?

There is no "rollback" in Redis. MVCC or something like that is not
part of the implementation.

[Thomas Fritz]

OK, but what about MULTI, EXEC and DISCARD?
http://redis.io/topics/transactions

---
Thomas FRITZ
web http://fritzthomas.com
twitter http://twitter.com/thomasf

2011/10/27 Pedro Melo <me...@simplicidade.org>:

[Salvatore Sanfilippo]

On Thu, Oct 27, 2011 at 1:21 PM, Thomas Fritz <frit...@gmail.com> wrote:
> OK, but what about MULTI, EXEC and DISCARD?
> http://redis.io/topics/transactions

If you lose the ability to read values from scripts most of the value
of scripting is lost.

When you are inside MULTI/EXEC you can't read stuff, you always get queued.

Salvatore

--

[Salvatore Sanfilippo]

Hi again,

I think I finally found a solution that makes me completely happy. It
is already implemented by the last commit in the unstable branch.

This is how it works:

* You can set the max execution limit as always.
* When that limit is reached Redis logs a warning, about a slow script detected.
* Redis starts re-entering the event loop once that condition is
detected, to accept new clients queries.
* Those queries will always be handled returning a -SLOWSCRIPT error,
that notifies the clients why the server is busy and can't reply.
* However the SHUTDOWN command is accepted in this condition to allow
the sysadmin to shutdown the server in a safe way without consistency
concerns.

I'm very happy with this solution, I hope you like it as well.

Salvatore

[Pedro Melo]

On Thu, Oct 27, 2011 at 12:21 PM, Thomas Fritz <frit...@gmail.com> wrote:
> OK, but what about MULTI, EXEC and DISCARD?
> http://redis.io/topics/transactions

What about it? It doesn't do rollbacks. You can't even find that word
on the page.

Its called a transaction because all commands are executed or none at
all, and with WATCH you can make sure that some precondition you based
your MULTI-EXEC'ed operations didn't change.

But it doesn't use speculative execution like a more traditional DBMS
that executes each command as it receives it, while keeping track of
undo information in a rollback segment somewhere. No. Redis accepts
all commands but it doesn't execute none of them until the final EXEC,
and then, atomically (as in without other clients operations in the
middle) it does all the ops that were sent after the MULTI.

[Pieter Noordhuis]

Exactly why I dislike the term "transaction" for MULTI/EXEC. The name
"atomic execution group" would be a better fit..

Cheers,
Pieter

[Andy McCurdy]

On Thursday, October 27, 2011 at 5:52 AM, Salvatore Sanfilippo wrote:

Hi again,

I think I finally found a solution that makes me completely happy. It

is already implemented by the last commit in the unstable branch.

This is how it works:

* You can set the max execution limit as always.

* When that limit is reached Redis logs a warning, about a slow script detected.

* Redis starts re-entering the event loop once that condition is

detected, to accept new clients queries.

* Those queries will always be handled returning a -SLOWSCRIPT error,

that notifies the clients why the server is busy and can't reply.

Perhaps we should have a more generic -BUSY. Might be able to reuse this in other situations where the server gets slowed down.

 

* However the SHUTDOWN command is accepted in this condition to allow

the sysadmin to shutdown the server in a safe way without consistency

concerns.

I know it's a bit more work, but a SCRIPT KILL <some id> command would be nicer. Larger DBs can take minutes to save/load. Seems overkill to shutdown the entire system to get rid of one connection. I certainly don't have to shutdown PostgreSQL or MySQL to kill a poorly written stored proc or long running query.

SCRIPT KILL would also make it easy for an external automated process to be able to respond to -SLOWSCRIPT errors so I don't have to wake up my sysadmin at 3:00AM :)

I'm very happy with this solution, I hope you like it as well.

Salvatore

On Wed, Oct 26, 2011 at 6:32 PM, Salvatore Sanfilippo <ant...@gmail.com> wrote:

On Wed, Oct 26, 2011 at 6:31 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:

In short: I'm in favor of not timing out scripts.

+1...

I want to just add this: the trap that used to terminate the script

instead of being removed at all can be turned into:

redisLog(REDIS_WARNING, "Hey... but we wanted to trust our users :(");

Makes sense?

--

Salvatore 'antirez' Sanfilippo

open source developer - VMware

http://invece.org

"We are what we repeatedly do. Excellence, therefore, is not an act,

but a habit." -- Aristotele

--

Salvatore 'antirez' Sanfilippo

open source developer - VMware

http://invece.org

"We are what we repeatedly do. Excellence, therefore, is not an act,

but a habit." -- Aristotele

[Jak Sprats]

maybe having a SCRIPT_LOG for scripts that run over X seconds and a
SCRIPT_SHUTDOWN for scripts running over XXXX seconds.

SCRIPT_LOG would on
A.) read-only scripts abort
B.) scripts-with-1+-writes log that the the script took over X
seconds and run to completion.

SCRIPT_SHUTDOWN would be a very high timeout and would shut down the
server.

Also on SHUTDOWN, the data really needs to be saved, so (SAVE
+SHUTDOWN) is needed.

And the real solution to all of this is to be responsible and (for non-
analytics) never ever write a script that takes longer than
50-100ms :)

On Oct 26, 10:08 am, Salvatore Sanfilippo <anti...@gmail.com> wrote:
> On Wed, Oct 26, 2011 at 5:45 PM, Louis-Philippe Perron
>

> <lpper...@gmail.com> wrote:
> > Why not set sensible defaults and allow redis.conf overrides?
> > like having a "SCRIPT_TIMEOUT" to set duration and
> > a "SCRIPT_SHUTDOWN" to set if server would shutdown on script fail.
>
> Hi Louis-Philippe, I think not shutting down is not an option if the
> script already did even a single write, otherwise the data set is left
> into an inconsistency state.
>
> Salvatore
>
>
>
> > L-P

> > On Wed, Oct 26, 2011 at 11:25 AM, Salvatore Sanfilippo <anti...@gmail.com>

> > wrote:
>
> >> Hi again,
>
> >> I slept on this, and I think we are going to be something rather extreme.
>
> >> Let alone that we end with problems in the AOF/replication link, but
> >> the biggest problem is that the scripting contract is broken if we
> >> abort the script half-way, since the application has no way to know
> >> what was done and what was not: in short this breaks scripts
> >> consistency.
>
> >> So this is my idea: if a script that has done no writes at all so far
> >> goes over the setup max time we abort it.
> >> If a script that already sent even a single write command goes after
> >> the max time, we need to call SHUTDOWN and terminate the server.
>
> >> The above solution can appear bold at first, but it is the *only* way
> >> to preserve consistency.
> >> Also:
>
> >> 1) Scripts should never run usually for a long time. This timeout
> >> should be set to two order of magnitudes bigger value compared to what
> >> the max script execution time should be.
> >> 2) Scripts usually run a lot of time because of bugs, for debugging it
> >> is not going to be a big problem that the server shuts down.
>
> >> I really think we don't have other options but would love to get some
> >> feedback on that.
>
> >> Salvatore
>

> >> On Tue, Oct 25, 2011 at 8:05 PM, Pieter Noordhuis <pcnoordh...@gmail.com>

[Jak Sprats]

Hi Salvatore,

this looks pretty simple/consistent/predictable, and it will allow
people to track down the slow script in the logs and clients will not
hang terribly if they implement some logic to handle -SLOWSCRIPT.

Now we need to come up w/ proper behavior for clients that get -
SLOWSCRIPT responses ... retry, back-off+retry, etc... something like
that probably.

- jak

On Oct 27, 6:52 am, Salvatore Sanfilippo <anti...@gmail.com> wrote:
> Hi again,
>
> I think I finally found a solution that makes me completely happy. It
> is already implemented by the last commit in the unstable branch.
>
> This is how it works:
>
> * You can set the max execution limit as always.
> * When that limit is reached Redis logs a warning, about a slow script detected.
> * Redis starts re-entering the event loop once that condition is
> detected, to accept new clients queries.
> * Those queries will always be handled returning a -SLOWSCRIPT error,
> that notifies the clients why the server is busy and can't reply.
> * However the SHUTDOWN command is accepted in this condition to allow
> the sysadmin to shutdown the server in a safe way without consistency
> concerns.
>
> I'm very happy with this solution, I hope you like it as well.
>
> Salvatore
>
>
>

> On Wed, Oct 26, 2011 at 6:32 PM, Salvatore Sanfilippo <anti...@gmail.com> wrote:

[Pedro Melo]

Hi,

On Thu, Oct 27, 2011 at 6:05 PM, Pieter Noordhuis <pcnoo...@gmail.com> wrote:
> Exactly why I dislike the term "transaction" for MULTI/EXEC. The name
> "atomic execution group" would be a better fit..

+1.

[ivan babrou]

On 27 October 2011 21:17, Andy McCurdy <sed...@gmail.com> wrote:

On Thursday, October 27, 2011 at 5:52 AM, Salvatore Sanfilippo wrote:

Hi again,

I think I finally found a solution that makes me completely happy. It

is already implemented by the last commit in the unstable branch.

This is how it works:

* You can set the max execution limit as always.

* When that limit is reached Redis logs a warning, about a slow script detected.

* Redis starts re-entering the event loop once that condition is

detected, to accept new clients queries.

* Those queries will always be handled returning a -SLOWSCRIPT error,

that notifies the clients why the server is busy and can't reply.

Perhaps we should have a more generic -BUSY. Might be able to reuse this in other situations where the server gets slowed down.

+1 for BUSY, it's more generic name.

 

* However the SHUTDOWN command is accepted in this condition to allow

the sysadmin to shutdown the server in a safe way without consistency

concerns.

I know it's a bit more work, but a SCRIPT KILL <some id> command would be nicer. Larger DBs can take minutes to save/load. Seems overkill to shutdown the entire system to get rid of one connection. I certainly don't have to shutdown PostgreSQL or MySQL to kill a poorly written stored proc or long running query.

SCRIPT KILL would also make it easy for an external automated process to be able to respond to -SLOWSCRIPT errors so I don't have to wake up my sysadmin at 3:00AM :)

Shutdown is real overhead. Especially with cluster and big databases. Maybe there should be KILL not only for scripts? If database is in swap and tries to load data into ram it's really slow. If every command and connection has an id there's a way to kill long requests. In that case they must return error.

--
Regards, Ian Babrou
http://bobrik.name http://twitter.com/ibobrik skype:i.babrou

[Salvatore Sanfilippo]

On Thu, Oct 27, 2011 at 7:38 PM, Jak Sprats <jaks...@gmail.com> wrote:
> Now we need to come up w/ proper behavior for clients that get -
> SLOWSCRIPT responses ... retry, back-off+retry, etc... something like
> that probably.

Hey Jak, I would simply return the error to the caller, as the -BUSY
(was SLOWSCRIPT) stuff is really a critical condition that should
never happen in production if the code is sane. The default is now
five seconds but I can make it even bigger, so that this will happen
only in pathological conditions.

Cheers,
Salvatore

[Salvatore Sanfilippo]

On Thu, Oct 27, 2011 at 7:17 PM, Andy McCurdy <sed...@gmail.com> wrote:

> Perhaps we should have a more generic -BUSY. Might be able to reuse this in
> other situations where the server gets slowed down.

Good idea Andy, done.

> I know it's a bit more work, but a SCRIPT KILL <some id> command would be

I would be more than willing to do the work, the problem is that this
breaks script semantics, you can't know how much of its work the
script was able to do, so this will leave the database in a possibly
inconsistent state.
That's why we can't stop it, otherwise implementing script kill would
be trivial. Actually *it is* trivial since no ID at all is needed,
there can be only a single script at time.

Cheers,
Salvatore

[Salvatore Sanfilippo]

This was one of the idea I evaluated, but ended excluding it as
auto-shutdown is really bad.

After all there will be people using Redis "computationally" that will
run log scripts for a reason, they have just one client.
Or to do big schema migrations.

They'll be very well served with the current behavior since the script
will not terminate (if they don't send SHUTDOWN), but at the same time
we are serving all the other users with a buggy script well, since
they'll be both the error reported by clients to understand what is
happening, and can select if let the script run or if kill the server,
restart it, and fix the program.

Cheers,
Salvatore

[Andy McCurdy]

On Thursday, October 27, 2011 at 12:00 PM, Salvatore Sanfilippo wrote:

On Thu, Oct 27, 2011 at 7:17 PM, Andy McCurdy <sed...@gmail.com> wrote:

Perhaps we should have a more generic -BUSY. Might be able to reuse this in

other situations where the server gets slowed down.

Good idea Andy, done.

I know it's a bit more work, but a SCRIPT KILL <some id> command would be

I would be more than willing to do the work, the problem is that this

breaks script semantics, you can't know how much of its work the

script was able to do, so this will leave the database in a possibly

inconsistent state.

But SHUTDOWN does an implicit SAVE. Any changes the script has already made to would get saved to disk, right? It seems like SHUTDOWN would have the same problem that killing the script would, leaving things in an inconsistent state.

 

That's why we can't stop it, otherwise implementing script kill would

be trivial. Actually *it is* trivial since no ID at all is needed,

there can be only a single script at time.

Cheers,

Salvatore

--

Salvatore 'antirez' Sanfilippo

open source developer - VMware

http://invece.org

"We are what we repeatedly do. Excellence, therefore, is not an act,

but a habit." -- Aristotele

[Salvatore Sanfilippo]

On Thu, Oct 27, 2011 at 10:29 PM, Andy McCurdy <sed...@gmail.com> wrote:

> But SHUTDOWN does an implicit SAVE. Any changes the script has already made
> to would get saved to disk, right? It seems like SHUTDOWN would have the
> same problem that killing the script would, leaving things in an
> inconsistent state.

That's true, with RDB we are basically out of luck, but at least with
AOF with SHUTDOWN we are safe and the DB is left in a consistent way.
With RDB there is this problem, maybe when SHUTDOWN is sent in this
condition better avoiding to SAVE at all?

[Salvatore Sanfilippo]

Ok that's my revisited proposal.

Everything like now, but:

1) SCRIPT KILL will work if a script did no write at all so far. This
is sensible as it saves the user from a restart of Redis if the issue
is just a read-only script trapped into an infinite loop.
2) SHUTDOWN when called in that context will not save the RDB at all.
Alternatively we can add an ABORT command just for that if it is more
sensible to make the user understand that this is not going to save
the RDB file.
3) Ability to call SAVE when the script is busy? The user may still
want a non consistent DB instead of loosing the altest queries.

Suggestions? Thanks.
Salvatore

[Andy McCurdy]

On Oct 28, 2011, at 5:38 AM, Salvatore Sanfilippo <ant...@gmail.com> wrote:

> Ok that's my revisited proposal.
>
> Everything like now, but:
>
> 1) SCRIPT KILL will work if a script did no write at all so far. This
> is sensible as it saves the user from a restart of Redis if the issue
> is just a read-only script trapped into an infinite loop.

How about an optional parameter to kill the script even if writes have
been made? SCRIPT KILL FORCE maybe? That would leave the DB
inconsistent, but not shutdown the server.

> 2) SHUTDOWN when called in that context will not save the RDB at all.
> Alternatively we can add an ABORT command just for that if it is more
> sensible to make the user understand that this is not going to save
> the RDB file.

Maybe SHUTDOWN NOSAVE?

> 3) Ability to call SAVE when the script is busy? The user may still
> want a non consistent DB instead of loosing the altest queries.
>

Don't need this with script kill force.

[Jan Oberst]

On Oct 27, 1:28 pm, Jak Sprats <jakspr...@gmail.com> wrote:
> maybe having a SCRIPT_LOG for scripts that run over X seconds and a
> SCRIPT_SHUTDOWN for scripts running over XXXX seconds.

I think, this is a great idea. Just like with the slowlog, I'd be
interested in seeing which scripts run the longest.

>
> SCRIPT_LOG would on
>  A.) read-only scripts abort
>  B.) scripts-with-1+-writes log that the the script took over X
> seconds and run to completion.

I definitely think, Redis should finish the completion if a write has
happened. Maybe add a config setting to tweak the behavior for read-
only scripts.

> SCRIPT_SHUTDOWN would be a very high timeout and would shut down the
> server.
>
> Also on SHUTDOWN, the data really needs to be saved, so (SAVE
> +SHUTDOWN) is needed.

Wouldn't a synchronous SAVE suffice in this example? If the script is
running for too long, the server blocks anyways. And after the SAVE
the database should be on disk in a consistent form.

> And the real solution to all of this is to be responsible and (for non-
> analytics) never ever write a script that takes longer than
> 50-100ms :)

Another problem one might run into is infinite loops, or loops that
take far too long than one imagined. Too many times I have pressed ctrl
+c because my script would've run for 10 years. As a responsible
programmer, I would never run those on a production machine, but it
would still be a pain to reload the dev server every time something
like this happens :)

Jan

[Salvatore Sanfilippo]

On Fri, Oct 28, 2011 at 4:17 PM, Andy McCurdy <sed...@gmail.com> wrote:
> On Oct 28, 2011, at 5:38 AM, Salvatore Sanfilippo <ant...@gmail.com> wrote:
>
>> Ok that's my revisited proposal.
>>
>> Everything like now, but:
>>
>> 1) SCRIPT KILL will work if a script did no write at all so far. This
>> is sensible as it saves the user from a restart of Redis if the issue
>> is just a read-only script trapped into an infinite loop.
>
> How about an optional parameter to kill the script even if writes have
> been made? SCRIPT KILL FORCE maybe? That would leave the DB
> inconsistent, but not shutdown the server.

So you are proposing something like that:

* We have a busy script that have never been doing writes yet: SCRIPT
KILL -> OK.
* We have a busy script that already performed at least one write
operaton -> SCRIPT KILL -> "ERR script already wrote so terminating it
may result in inconsistent database, use SCRIPT KILL FORCE if you know
what you are doing."

It makes sense but the problem I see here is about the "user
interface" we want to have with the programmer.
In this mailing list our context is biased, we know about what we are
talking about. Users are not all Redis experts, so they'll pick SCRIPT
KILL FORCE just since the previous command did not worked. They don't
exactly know what it will do, and why it may be a bad idea IMHO.

So I'm currently more biased towards not allowing the termination of
scripts that wrote on the DB.

>> 2) SHUTDOWN when called in that context will not save the RDB at all.
>> Alternatively we can add an ABORT command just for that if it is more
>> sensible to make the user understand that this is not going to save
>> the RDB file.
>
> Maybe SHUTDOWN NOSAVE?

We already saw a similar, reversed, feature request: SHUTDOWN
FORCESAVE or alike (there is an issue open about it), since SHUTDOWN
may not save if no save point is configured.

Still ABORT seems more clear to me, since currently the only
difference is that it does not save, but the general semantics is:
close the server without trying to be an hero saving or doing any
other cleanup. Just delete the pid and exit.

>> 3) Ability to call SAVE when the script is busy? The user may still
>> want a non consistent DB instead of loosing the altest queries.
>>
>
> Don't need this with script kill force.

Yep, I actually don't think it is a good idea at all to let the user
to recover from that condition performing a SAVE.
Either it cared about durability and has AOF enabled, or he was ok
with a N-minutes snapshot so there is the old file.

I don't want to allow in any case an inconsistent DB to survive.

Thanks for your help, very appreciated and useful in investigating
better this important aspect.

Salvatore

[Andy McCurdy]

On Friday, October 28, 2011 at 8:56 AM, Salvatore Sanfilippo wrote:

On Fri, Oct 28, 2011 at 4:17 PM, Andy McCurdy <sed...@gmail.com> wrote:

On Oct 28, 2011, at 5:38 AM, Salvatore Sanfilippo <ant...@gmail.com> wrote:

Ok that's my revisited proposal.

Everything like now, but:

1) SCRIPT KILL will work if a script did no write at all so far. This

is sensible as it saves the user from a restart of Redis if the issue

is just a read-only script trapped into an infinite loop.

How about an optional parameter to kill the script even if writes have

been made? SCRIPT KILL FORCE maybe? That would leave the DB

inconsistent, but not shutdown the server.

So you are proposing something like that:

* We have a busy script that have never been doing writes yet: SCRIPT

KILL -> OK.

* We have a busy script that already performed at least one write

operaton -> SCRIPT KILL -> "ERR script already wrote so terminating it

may result in inconsistent database, use SCRIPT KILL FORCE if you know

what you are doing."

It makes sense but the problem I see here is about the "user

interface" we want to have with the programmer.

In this mailing list our context is biased, we know about what we are

talking about. Users are not all Redis experts, so they'll pick SCRIPT

KILL FORCE just since the previous command did not worked. They don't

exactly know what it will do, and why it may be a bad idea IMHO.

Ya, that's exactly what I'm suggesting. I was also coming from the user interface standpoint :)

IMO, having similar, several ambiguously named commands is less intuitive than overloading SCRIPT KILL with an optional FORCE parameter.

Also, we'd probably want a SCRIPT INFO or similar command that will show the script that's currently running. Imagine a scenario where several scripts are installed but one of them is taking a long time. Without SCRIPT INFO, it'd be difficult to tell which one is the culprit.

 

2) SHUTDOWN when called in that context will not save the RDB at all.

Alternatively we can add an ABORT command just for that if it is more

sensible to make the user understand that this is not going to save

the RDB file.

Maybe SHUTDOWN NOSAVE?

We already saw a similar, reversed, feature request: SHUTDOWN

FORCESAVE or alike (there is an issue open about it), since SHUTDOWN

may not save if no save point is configured.

Still ABORT seems more clear to me, since currently the only

difference is that it does not save, but the general semantics is:

close the server without trying to be an hero saving or doing any

other cleanup. Just delete the pid and exit.

Same argument as above. SHUTDOWN NOSAVE is pretty clear what it's about to do. Just looking at SHUTDOWN and ABORT, I'd have to read the docs to figure it out.

[Nathan Fritz]

How does a blocking redis.call like BRPOP fit into all of this?
At the moment, it doesn't appear to block.  This is a problem for me.

-Fritzy

[Salvatore Sanfilippo]

On Sat, Oct 29, 2011 at 6:38 AM, Nathan Fritz <natha...@gmail.com> wrote:
> How does a blocking redis.call like BRPOP fit into all of this?
> At the moment, it doesn't appear to block.  This is a problem for me.

Blocking from LUA will not work at all, the blocking commands will
actually return an error in the final version...

Cheers,
Salvatore

> -Fritzy