mediocregopher / radix.v2

Redis client for Go
http://godoc.org/github.com/mediocregopher/radix.v2
MIT License
433 stars 92 forks source link

Implement more convenient support for transactions? #59

Closed jootuom closed 6 years ago

jootuom commented 7 years ago

Hello,

The documentation for this library does not seem to offer any guidance at all on doing transactions as far as I can see. Pipelining is mentioned, but pipelining does not equate a transaction automatically, and pipelining by itself does not guarantee that the pipelined commands are executed atomically. You need transactions for that.

The Python Redis client library automatically/transparently uses transactions when you use pipelines, but that doesn't seem to be the case with this library?

The upstream documentation linked above explains it all, but to summarize:

Transactions are initiated by issuing the "MULTI" command, after which any other issued command will only return a "QUEUED" response (or error) until you issue the "EXEC" command, after which the server will return the response data to every queued command at once, or fail. So, transactions also imply special semantics for error handling.

With current functionality, transactions seem to be very awkward to use:

conn.PipeAppend("multi")
conn.PipeAppend("lrange", "list1", 0, -1)
conn.PipeAppend("lrange", "list2", 0, -1)
conn.PipeAppend("del", "list1")
conn.PipeAppend("del", "list2")
conn.PipeAppend("exec")

// this is completely redundant
for i := 0; i < 5; i++ {
    if err := conn.PipeResp().Err; err != nil {
        // do whatever
    }
}

resp, err := conn.PipeResp.Array()

if err != nil {
    //  something
}

// resp will now contain the result to every command

Surely there's a better way to handle this?

Maybe special transaction functionality should be implemented where you only have to fetch the response (and error) once, since the entire transaction will succeed or fail and checking every error is redundant.

mediocregopher commented 7 years ago

Hi @jootuom, thanks for submitting the issue! Sorry it took so long to respond, I've been out of town, but here's my thoughts:

The Python Redis client library automatically/transparently uses transactions when you use pipelines, but that doesn't seem to be the case with this library?

I would very much disagree with that approach, my view is that almost nothing should be done transparently, as that often causes subtle unexpected behavior in unexpected situations.

Maybe special transaction functionality should be implemented where you only have to fetch the response (and error) once, since the entire transaction will succeed or fail and checking every error is redundant.

I wouldn't be opposed to adding something to this effect to the util package, but I'm curious as to what it would actually look like. I imagine there's have to be some kind of Transaction struct which contains a Client internally and looks similar to the pipeline code. It'd also need to allow for WATCH, UNWATCH, and DISCARD commands. If you'd like to take a stab at implementing it I'd be interested in working with you on it :)


As a side note, if you haven't checked out the lua functionality in redis I'd highly recommend you do so. antirez (redis' creator and lead maintainer) has said before that he's even considered deprecating transactions in favor of the lua functionality, as it offers everything transactions do and more, with better performance to boot.

jootuom commented 7 years ago

Yeah I think this should essentially work just like pipelining. I actually just started with Go so I'm not really completely sure what the best way to implement this would be.

I know the Lua functionality exists but I've never felt the need to use that, transactions have served me just fine for now. To me that feels more like something you would use when you need to do really custom stuff, while transactions are nicely "built in". Sometimes you can't accomplish something with one command but you can with two (but then you want to do it atomically, though I guess the possibility of collisions is small) and transactions suffice.

mediocregopher commented 6 years ago

Hey @jootuom just wanted to let you know I'm going to go ahead and close this issue. I've focused all my efforts on radix.v3, so any further work on transactions (if there is any) would happen there

I know the Lua functionality exists but I've never felt the need to use that, transactions have served me just fine for now.

I'd really encourage you to check it out anyway, you never know what kinds of doors it would open up for you, even if it means a bit of a learning curve. antirez has also made some allusions that lua could replace transactions entirely, so you may have to learn it one way or the other.