Skip to content

Commit

Permalink
Refactor Tx using Pipeline to implement Cmdable interface.
Browse files Browse the repository at this point in the history
  • Loading branch information
vmihailenco committed Oct 13, 2016
1 parent 3490ff5 commit 20bc3ec
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 69 deletions.
4 changes: 2 additions & 2 deletions cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ var _ = Describe("ClusterClient", func() {
return err
}

_, err = tx.MultiExec(func() error {
tx.Set(key, strconv.FormatInt(n+1, 10), 0)
_, err = tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Set(key, strconv.FormatInt(n+1, 10), 0)
return nil
})
return err
Expand Down
4 changes: 2 additions & 2 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ func ExampleClient_Watch() {
return err
}

_, err = tx.MultiExec(func() error {
tx.Set(key, strconv.FormatInt(n+1, 10), 0)
_, err = tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Set(key, strconv.FormatInt(n+1, 10), 0)
return nil
})
return err
Expand Down
6 changes: 3 additions & 3 deletions pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ var _ = Describe("pool", func() {
Expect(pool.Len()).To(Equal(pool.FreeLen()))
})

It("srespect max size on multi", func() {
It("respects max size on multi", func() {
perform(1000, func(id int) {
var ping *redis.StatusCmd

err := client.Watch(func(tx *redis.Tx) error {
cmds, err := tx.MultiExec(func() error {
ping = tx.Ping()
cmds, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
ping = pipe.Ping()
return nil
})
Expect(err).NotTo(HaveOccurred())
Expand Down
4 changes: 2 additions & 2 deletions race_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ var _ = Describe("races", func() {
num, err := strconv.ParseInt(val, 10, 64)
Expect(err).NotTo(HaveOccurred())

cmds, err := tx.MultiExec(func() error {
tx.Set("key", strconv.FormatInt(num+1, 10), 0)
cmds, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Set("key", strconv.FormatInt(num+1, 10), 0)
return nil
})
Expect(cmds).To(HaveLen(1))
Expand Down
4 changes: 2 additions & 2 deletions redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ var _ = Describe("Client", func() {

It("should close Tx without closing the client", func() {
err := client.Watch(func(tx *redis.Tx) error {
_, err := tx.MultiExec(func() error {
tx.Ping()
_, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Ping()
return nil
})
return err
Expand Down
75 changes: 29 additions & 46 deletions tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
// Redis transaction failed.
const TxFailedErr = internal.RedisError("redis: transaction failed")

var errDiscard = internal.RedisError("redis: Discard can be used only inside Exec")

// Tx implements Redis transactions as described in
// http://redis.io/topics/transactions. It's NOT safe for concurrent use
// by multiple goroutines, because Exec resets list of watched keys.
Expand All @@ -22,10 +20,11 @@ type Tx struct {
statefulCmdable
baseClient

cmds []Cmder
closed bool
}

var _ Cmdable = (*Tx)(nil)

func (c *Client) newTx() *Tx {
tx := Tx{
baseClient: baseClient{
Expand Down Expand Up @@ -53,14 +52,6 @@ func (c *Client) Watch(fn func(*Tx) error, keys ...string) error {
return retErr
}

func (c *Tx) Process(cmd Cmder) error {
if c.cmds == nil {
return c.baseClient.Process(cmd)
}
c.cmds = append(c.cmds, cmd)
return nil
}

// close closes the transaction, releasing any open resources.
func (c *Tx) close() error {
if c.closed {
Expand Down Expand Up @@ -98,16 +89,16 @@ func (c *Tx) Unwatch(keys ...string) *StatusCmd {
return cmd
}

// Discard discards queued commands.
func (c *Tx) Discard() error {
if c.cmds == nil {
return errDiscard
func (c *Tx) Pipeline() *Pipeline {
pipe := Pipeline{
exec: c.exec,
}
c.cmds = c.cmds[:1]
return nil
pipe.cmdable.process = pipe.Process
pipe.statefulCmdable.process = pipe.Process
return &pipe
}

// MultiExec executes all previously queued commands in a transaction
// Pipelined executes commands queued in the fn in a transaction
// and restores the connection state to normal.
//
// When using WATCH, EXEC will execute commands only if the watched keys
Expand All @@ -116,36 +107,29 @@ func (c *Tx) Discard() error {
// Exec always returns list of commands. If transaction fails
// TxFailedErr is returned. Otherwise Exec returns error of the first
// failed command or nil.
func (c *Tx) MultiExec(fn func() error) ([]Cmder, error) {
if c.closed {
return nil, pool.ErrClosed
}

c.cmds = []Cmder{NewStatusCmd("MULTI")}
if err := fn(); err != nil {
return nil, err
}
c.cmds = append(c.cmds, NewSliceCmd("EXEC"))

cmds := c.cmds
c.cmds = nil
func (c *Tx) Pipelined(fn func(*Pipeline) error) ([]Cmder, error) {
return c.Pipeline().pipelined(fn)
}

if len(cmds) == 2 {
return []Cmder{}, nil
func (c *Tx) exec(cmds []Cmder) error {
if c.closed {
return pool.ErrClosed
}

// Strip MULTI and EXEC commands.
retCmds := cmds[1 : len(cmds)-1]

cn, _, err := c.conn()
if err != nil {
setCmdsErr(retCmds, err)
return retCmds, err
setCmdsErr(cmds, err)
return err
}

err = c.execCmds(cn, cmds)
multiExec := make([]Cmder, 0, len(cmds)+2)
multiExec = append(multiExec, NewStatusCmd("MULTI"))
multiExec = append(multiExec, cmds...)
multiExec = append(multiExec, NewSliceCmd("EXEC"))

err = c.execCmds(cn, multiExec)
c.putConn(cn, err, false)
return retCmds, err
return err
}

func (c *Tx) execCmds(cn *pool.Conn, cmds []Cmder) error {
Expand All @@ -155,12 +139,11 @@ func (c *Tx) execCmds(cn *pool.Conn, cmds []Cmder) error {
return err
}

statusCmd := NewStatusCmd()

// Omit last command (EXEC).
cmdsLen := len(cmds) - 1

// Parse queued replies.
statusCmd := cmds[0]
for i := 0; i < cmdsLen; i++ {
if err := statusCmd.readReply(cn); err != nil {
setCmdsErr(cmds[1:len(cmds)-1], err)
Expand All @@ -183,18 +166,18 @@ func (c *Tx) execCmds(cn *pool.Conn, cmds []Cmder) error {
return err
}

var firstCmdErr error
var firstErr error

// Parse replies.
// Loop starts from 1 to omit MULTI cmd.
for i := 1; i < cmdsLen; i++ {
cmd := cmds[i]
if err := cmd.readReply(cn); err != nil {
if firstCmdErr == nil {
firstCmdErr = err
if firstErr == nil {
firstErr = err
}
}
}

return firstCmdErr
return firstErr
}
24 changes: 12 additions & 12 deletions tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ var _ = Describe("Tx", func() {
return err
}

_, err = tx.MultiExec(func() error {
tx.Set(key, strconv.FormatInt(n+1, 10), 0)
_, err = tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Set(key, strconv.FormatInt(n+1, 10), 0)
return nil
})
return err
Expand Down Expand Up @@ -65,10 +65,10 @@ var _ = Describe("Tx", func() {

It("should discard", func() {
err := client.Watch(func(tx *redis.Tx) error {
cmds, err := tx.MultiExec(func() error {
tx.Set("key1", "hello1", 0)
tx.Discard()
tx.Set("key2", "hello2", 0)
cmds, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Set("key1", "hello1", 0)
pipe.Discard()
pipe.Set("key2", "hello2", 0)
return nil
})
Expect(err).NotTo(HaveOccurred())
Expand All @@ -88,7 +88,7 @@ var _ = Describe("Tx", func() {

It("should exec empty", func() {
err := client.Watch(func(tx *redis.Tx) error {
cmds, err := tx.MultiExec(func() error { return nil })
cmds, err := tx.Pipelined(func(*redis.Pipeline) error { return nil })
Expect(err).NotTo(HaveOccurred())
Expect(cmds).To(HaveLen(0))
return err
Expand All @@ -104,9 +104,9 @@ var _ = Describe("Tx", func() {
const N = 20000

err := client.Watch(func(tx *redis.Tx) error {
cmds, err := tx.MultiExec(func() error {
cmds, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
for i := 0; i < N; i++ {
tx.Incr("key")
pipe.Incr("key")
}
return nil
})
Expand Down Expand Up @@ -135,8 +135,8 @@ var _ = Describe("Tx", func() {

do := func() error {
err := client.Watch(func(tx *redis.Tx) error {
_, err := tx.MultiExec(func() error {
tx.Ping()
_, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
pipe.Ping()
return nil
})
return err
Expand All @@ -162,7 +162,7 @@ var _ = Describe("Tx", func() {

do := func() error {
err := client.Watch(func(tx *redis.Tx) error {
_, err := tx.MultiExec(func() error {
_, err := tx.Pipelined(func(pipe *redis.Pipeline) error {
return nil
})
return err
Expand Down

0 comments on commit 20bc3ec

Please sign in to comment.