Handling of Pipeline Errors

[Andy McCurdy]

Hey everyone,

A redis-py user recently encountered an error with the way errors are handled in the pipeline implementation. Fixing the error is trivial, but I'm curious if there's a standard way client libs should be handling errors in pipelines.

In pipelines that are wrapped in MULTI/EXEC, there's two times that errors can happen -- parse time and exec time:

* Parse time errors include any error that happens when the command is sent to the server to be QUEUED.
* Exec time errors are only found when the command is actually executed at EXEC time (such as performing a list operation on a key holding a string value)

Currently redis-py handles exec time errors by placing the raised exception instance into the list of returned values from the EXEC command. For instance:

>>> redis.set('foo', '1')
>>> pipe = redis.pipeline()
>>> pipe.lpush('foo', 'some-value')
>>> pipe.set('bar', '1')
>>> pipe.execute()
[redis.exceptions.ResponseError('Operation against a key holding the wrong kind of value'), True]
# but the SET to 'bar' still got executed
>>> redis.get('bar')
'1'

My question is: should parse time errors be handled in the same way? We have the opportunity to detect any parse time error before EXEC is run, potentially DISCARDing all the statements and raise the error immediately. Alternatively parse time errors could be treated identical to exec time errors and exceptions could be inserted into EXEC's return value just like above.

Thoughts?

-andy

[Salvatore Sanfilippo]

Hi Andy,

thanks for raising this issue, I think it is an interesting topic to
discuss and one where guidelines are a good idea as it's not a matter
of API or language, but about semantics.

I would always raise an exception on parse-time errors, because
clearly there is no point into executing a transaction that will have
missing commands because they are misspelled or alike. So unless the
user traps the exception in some language-specific way, the error will
stop the execution of the program and the transaction will not be
executed at all.

Unfortunately once you add pipelining into the mix things are not as
simple as they should be, because:

1) If you send MULTI / commands / EXEC in the same pipeline you can't
prevent the EXEC even if you detected errors.
2) If you split the pipeline sending "MULTI / commands" before, read
the reply, and than EXEC, you can handle it correctly, but this means
to pay two times the round trip time, that's not cool.

So since parse-time errors should only happen on *syntax errors*, I think that:

1) If the library does not use pipelining for MULTI/EXEC the
parse-time error should raised ASAP, if reply is not "QUEUED".
2) If the library uses pipelining it's ok to raise the error after
sending the EXEC, but the error raised should be the first parse-time
error encountered if any.

After all, even in an ACID SQL system, if there is a syntax error in
the query, no body can save the user from a disaster, but this should
only happen in development environments.

About exec-time errors, I think this category of errors should also
raise an exception that should stop the execution of the program by
default, because in this stage errors can happen only for severe
conditions like wrong type (problem in the application logic), or out
of memory in Redis if it's configured with maxmemory, and other
similar things that are not expected usually.

However I think it's a good idea if, optionally, libraries have an API
that allows to just call EXEC without raising errors but just
returning the array of replies, that may include errors elements
inside, with a way to test for errors. Like:

result = redis.exec(trap_errors = true)
if result[3].is_error() ...

in this way the default behaviour is the safest, but the library still
delivers all the power to handle everything by hand if needed (think
about writing a shell that returns an array of replies including
errors for MULTI/EXEC blocks).

Cheers,
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 'antirez' Sanfilippo
open source developer - VMware
http://invece.org

Beauty is more important in computing than anywhere else in technology
because software is so complicated. Beauty is the ultimate defence
against complexity.
— David Gelernter

[Andy McCurdy]

Great, thanks. My only concern about raising errors at EXEC time is that you lose the results of any successful command that completed in the pipeline. This probably isn't a terrible thing, and raising does seem more appropriate to let people know that there is in fact a bug in their code.

-andy

[Ronan Amicel]

On Fri, Oct 5, 2012 at 7:02 PM, Andy McCurdy <sed...@gmail.com> wrote:
> Great, thanks. My only concern about raising errors at EXEC time is that you lose the results of any successful command that completed in the pipeline. This probably isn't a terrible thing, and raising does seem more appropriate to let people know that there is in fact a bug in their code.

Maybe the default could be to raise an exception, which avoids silent failures:

>>> redis.set('foo', '1')
>>> pipe = redis.pipeline()
>>> pipe.lpush('foo', 'some-value')
>>> pipe.set('bar', '1')
>>> pipe.execute()

Traceback (most recent call last):
(...)
ResponseError: Operation against a key holding the wrong kind of value

And optionally, you could use a parameter to execute() to tell the
library that you want to get the array of results and handle any
errors yourself, e.g. :

>>> redis.set('foo', '1')
>>> pipe = redis.pipeline()
>>> pipe.lpush('foo', 'some-value')
>>> pipe.set('bar', '1')

>>> pipe.execute(catch_errors=True)

[redis.exceptions.ResponseError('Operation against a key holding
the wrong kind of value'), True]

--
Ronan Amicel

«« Twitter overload?
»» Get your daily summary at http://focus.io/

[Andy McCurdy]

Yup, that's what I'm working on now.