Skip to content

Commit

Permalink
Change ReceiveMessage to not use Ping
Browse files Browse the repository at this point in the history
  • Loading branch information
vmihailenco committed Jul 24, 2018
1 parent c696191 commit 9bb7bb3
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 38 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

- Ring got new options called `HashReplicas` and `Hash`. It is recommended to set `HashReplicas = 1000` for better keys distribution between shards.
- Cluster client was optimized to use much less memory when reloading cluster state.
- ReceiveMessage is re-worked to not use ReceiveTimeout so it does not lose data when timeout occurres.
- PubSub.ReceiveMessage is re-worked to not use ReceiveTimeout so it does not lose data when timeout occurres. In most cases it is recommended to use PubSub.Channel instead.

## v6.12

Expand Down
20 changes: 6 additions & 14 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,26 +324,18 @@ func ExamplePubSub() {
pubsub := client.Subscribe("mychannel1")
defer pubsub.Close()

// Wait for subscription to be created before publishing message.
subscr, err := pubsub.ReceiveTimeout(time.Second)
if err != nil {
panic(err)
}
fmt.Println(subscr)

err = client.Publish("mychannel1", "hello").Err()
if err != nil {
panic(err)
}
// Go channel which receives messages.
ch := pubsub.Channel()

msg, err := pubsub.ReceiveMessage()
// Publish a message.
err := client.Publish("mychannel1", "hello").Err()
if err != nil {
panic(err)
}

msg := <-ch
fmt.Println(msg.Channel, msg.Payload)
// Output: subscribe: mychannel1
// mychannel1 hello
// Output: mychannel1 hello
}

func ExamplePubSub_Receive() {
Expand Down
54 changes: 31 additions & 23 deletions pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ type PubSub struct {

cmd *Cmd

pingOnce sync.Once
ping chan struct{}

chOnce sync.Once
ch chan *Message
ping chan struct{}
}

func (c *PubSub) init() {
Expand Down Expand Up @@ -326,8 +324,8 @@ func (c *PubSub) newMessage(reply interface{}) (interface{}, error) {
}

// ReceiveTimeout acts like Receive but returns an error if message
// is not received in time. This is low-level API and most clients
// should use ReceiveMessage instead.
// is not received in time. This is low-level API and in most cases
// Channel should be used instead.
func (c *PubSub) ReceiveTimeout(timeout time.Duration) (interface{}, error) {
if c.cmd == nil {
c.cmd = NewCmd()
Expand All @@ -349,28 +347,22 @@ func (c *PubSub) ReceiveTimeout(timeout time.Duration) (interface{}, error) {
}

// Receive returns a message as a Subscription, Message, Pong or error.
// See PubSub example for details. This is low-level API and most clients
// should use ReceiveMessage instead.
// See PubSub example for details. This is low-level API and in most cases
// Channel should be used instead.
func (c *PubSub) Receive() (interface{}, error) {
return c.ReceiveTimeout(0)
}

// ReceiveMessage returns a Message or error ignoring Subscription or Pong
// messages. It periodically sends Ping messages to test connection health.
// ReceiveMessage returns a Message or error ignoring Subscription and Pong
// messages. This is low-level API and in most cases Channel should be used
// instead.
func (c *PubSub) ReceiveMessage() (*Message, error) {
c.pingOnce.Do(c.initPing)
for {
msg, err := c.Receive()
if err != nil {
return nil, err
}

// Any message is as good as a ping.
select {
case c.ping <- struct{}{}:
default:
}

switch msg := msg.(type) {
case *Subscription:
// Ignore.
Expand All @@ -386,6 +378,7 @@ func (c *PubSub) ReceiveMessage() (*Message, error) {
}

// Channel returns a Go channel for concurrently receiving messages.
// It periodically sends Ping messages to test connection health.
// The channel is closed with PubSub. Receive* APIs can not be used
// after channel is created.
func (c *PubSub) Channel() <-chan *Message {
Expand All @@ -395,10 +388,12 @@ func (c *PubSub) Channel() <-chan *Message {

func (c *PubSub) initChannel() {
c.ch = make(chan *Message, 100)
c.ping = make(chan struct{}, 10)

go func() {
var errCount int
for {
msg, err := c.ReceiveMessage()
msg, err := c.Receive()
if err != nil {
if err == pool.ErrClosed {
close(c.ch)
Expand All @@ -411,16 +406,29 @@ func (c *PubSub) initChannel() {
continue
}
errCount = 0
c.ch <- msg

// Any message is as good as a ping.
select {
case c.ping <- struct{}{}:
default:
}

switch msg := msg.(type) {
case *Subscription:
// Ignore.
case *Pong:
// Ignore.
case *Message:
c.ch <- msg
default:
internal.Logf("redis: unknown message: %T", msg)
}
}
}()
}

func (c *PubSub) initPing() {
const timeout = 5 * time.Second

c.ping = make(chan struct{}, 10)
go func() {
const timeout = 5 * time.Second

timer := time.NewTimer(timeout)
timer.Stop()

Expand Down

0 comments on commit 9bb7bb3

Please sign in to comment.