From 20bc3ec5a69c72e9f223c6f1b2c1bde113938fab Mon Sep 17 00:00:00 2001 From: Vladimir Mihailenco Date: Thu, 13 Oct 2016 14:36:15 +0300 Subject: [PATCH] Refactor Tx using Pipeline to implement Cmdable interface. --- cluster_test.go | 4 +-- example_test.go | 4 +-- pool_test.go | 6 ++-- race_test.go | 4 +-- redis_test.go | 4 +-- tx.go | 75 +++++++++++++++++++------------------------------ tx_test.go | 24 ++++++++-------- 7 files changed, 52 insertions(+), 69 deletions(-) diff --git a/cluster_test.go b/cluster_test.go index 9639cf82..4a3dd7fa 100644 --- a/cluster_test.go +++ b/cluster_test.go @@ -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 diff --git a/example_test.go b/example_test.go index 1aef66fc..0769aa05 100644 --- a/example_test.go +++ b/example_test.go @@ -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 diff --git a/pool_test.go b/pool_test.go index 1adea5e8..607fa463 100644 --- a/pool_test.go +++ b/pool_test.go @@ -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()) diff --git a/race_test.go b/race_test.go index cf093f25..d6503eb2 100644 --- a/race_test.go +++ b/race_test.go @@ -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)) diff --git a/redis_test.go b/redis_test.go index 8d5f651a..8389e008 100644 --- a/redis_test.go +++ b/redis_test.go @@ -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 diff --git a/tx.go b/tx.go index 950c374f..2a459903 100644 --- a/tx.go +++ b/tx.go @@ -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. @@ -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{ @@ -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 { @@ -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 @@ -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) { +func (c *Tx) Pipelined(fn func(*Pipeline) error) ([]Cmder, error) { + return c.Pipeline().pipelined(fn) +} + +func (c *Tx) exec(cmds []Cmder) error { if c.closed { - return nil, pool.ErrClosed + return 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 - - if len(cmds) == 2 { - return []Cmder{}, nil - } - - // 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 { @@ -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) @@ -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 } diff --git a/tx_test.go b/tx_test.go index 03d220bb..9631e4cb 100644 --- a/tx_test.go +++ b/tx_test.go @@ -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 @@ -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()) @@ -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 @@ -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 }) @@ -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 @@ -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