Client has an buffer limit of 256 messages before failing

[centrifugal]

Client has an buffer limit of 256 messages before failing

[FZambia]

When I wrote this I thought that this can prevent blocking when slow client does not read from socket fast enough. Your last solution will help with arbitrary throughput - but what if send blocks and m will grow more and more? I don't have strong understanding if this blocking possible or not.

[klauspost]

client.send() blocking shouldn't be an issue, since sendMessages will always pull messages from c.messageChan.

But if c.sess.Send(msg) doesn't return, m will keep growing. What do you think would be the optimal solution? A 10 second (configurable) timeout on c.sess.Send(msg)?

[FZambia]

For raw websocket connection there is SetWriteDeadline (godoc) method available, but SockJS session has no such possibility.

[klauspost]

We could wrap message sending in a function:

func (c *client) sendTimeout(msg string) error {
  to := time.After(time.Second)
  sent := make(chan error)
  go func() {
      sent <- c.sess.Send(msg)
  }()
  select {
     case err := <- sent:
        return err
     case <- to:
        return ErrSendTimeout
  }
  panic("unreachable")
  return nil
}

The timeout length could be added to the client.

The thing that I don't really like about this solution is that we are spawning 2 goroutines to send a single message. Not a huge deal - goroutines are cheap, and it does seem like it is required.

I will make a PR proposal. It is a bit easier to discuss.

[FZambia]

Maybe the best way right now - make buffer larger and make it configurable and see how it goes? And then after writing some benchmark tests it will be more clear what's the right solution to this?

[klauspost]

I made a proposal in #23. After a few rewrites I think it is fairly unobtrusive, light on resources and rather easy to follow (except perhaps the ring queue implementation).

[FZambia]

Looks very interesting! Not sure this line necessary as clean method must be called anyway. There is old benchmark for Centrifuge - will try to run it after work to see memory and performance impact of these changes.