'broadcast' method in sockjs-tornado

[Chris]

Hi,

I have two questions regarding the 'broadcast' method in sockjs-tornado.

1. does the msg get encoded to JSON once for each recipient client? e.g. if there are 10 clients to send, then the same msg gets encoded 10 times? It seems to be the case here: https://github.com/mrjoes/sockjs-tornado/blob/master/sockjs/tornado/router.py#L193-201

2. if the connection to a client is broken when sending to that client, is there any way to 'capture' that broken connection instance? In other words, if I try to re-send after some failure which happens during the broadcast, how can I re-send only for the connection(s) that caused the failure?

Thanks!

[Serge S. Koval]

Hi,

1. No, it is doing encoding once. It just checks if particular transport expects packet in JSON encoding and does JSON encoding. So, it will be encoding per 10 clients at max.

2. There's no way to tell if connection silently died in broadcast() without checking if it was closed in connections on_close method. If connection died, there's no way you can resend message to it - client gone already.

If you need guaranteed delivery, check this thread: https://groups.google.com/forum/?fromgroups=#!topic/python-tornado/FYg4g2cAoQs

Serge.

[Chris]

Hi,

thanks for the confirmation. One more detail, if i call conn.close() immediately after the conn.send('blah') at the end of on_message(), will the msg always be delivered first before the connection is closed?

[Serge S. Koval]

For persistent transports (websocket, streaming, etc) - will be.

For polling transports: only if client waits for data from the server. If you close it in-between polls, client won't receive that last message.

This is default SockJS behavior, I raised this with SockJS devs and it is possible that new SockJS version will flush outgoing buffer for polling transports even if connection is considered closed.

Serge.

[Chris]

thank you for explaining this, and i hope the sockjs devs could make the behavior consistent across all protocols.

on the sockjs-tornado side, i wonder what would happen for this corner case:

first, on the server-side, i store an instance of the connection in a variable (let's say 'conn');

then, when the client sends a msg, connection breaks, on_close() method on the server-side is invoked.

a moment later, the server attempts to send a message using already-broken connection referenced by 'conn'.

so will the on_close() method on the server-side be invoked again? if not, what will happen instead?

The other behavior I'm trying to make sure is that, if the underlying connection is already broken, will calling send() method (from either client or server) *immediately* invoke on_close() method on both ends? Thanks.

[Serge S. Koval]

No, on_close is triggered only once. If you try to send something over closed connection - it will be silently ignored.

And no, send() won't invoke on_close immediately. sockjs-tornado handles disconnections (generic Tornado on_connection_close). If connection dies or you close server-side session, on_close will be triggered.

Serge.

[Chris]

We know that the underlying TCP sometimes does not die as soon as network errors happen but after some timeout. So in the case that network errors have already happened (which will eventually lead to disconnection), if we send another msg using this faulty connection, would that help us detect the problem sooner by causing the disconnection immediately?

[Serge S. Koval]

No, most likely it won't help.

Tornado will queue outgoing data in its internal buffers if OS will report that queue is full (due to TCP congestion prevention algorithm).

Proper way to detect that client gone - implement heartbeats.

Serge.

[Chris]

By implementing heartbeats, you mean to have the clients send a minimal msg to server every n seconds, and make the server to detect if (for any client) no msg is received for more than n seconds, and if so, assume that client is dead? if that is the case, it sounds like the server is going to do a lot of work, especially when n is small.

or is there more efficient implementation of heartbeats?

[Serge S. Koval]

That's pretty much how it works.

Socket.IO implements heartbeats and default value is 15 seconds. If there are more than 3 missed heartbeats, it will mark connection as closed. 

SockJS 0.4 will support heartbeats as well. Once it is released, I will add heartbeat support to sockjs-tornado.

Unfortunately, there's no way to see if client disconnected before OS closes connection without heartbeats.

Serge.

[Chris]

I see. So before SockJS 0.4 is released, I guess I have to implement the heartbeat checking on the server-side myself. Is this done by having a separate thread dedicated for continuously comparing each client's previous heartbeat timestamp and the current time?

Some suggestions would be very useful. thanks!

[Chris]

Hello Serge,

after some digging around tornadio2 and sockjs-tornado's source code, I wonder if the following is correct for adding heartbeat support in sockjs-tornado at application level (before sockjs 0.4.0 come out with heartbeat and before you implement it in sockjs-tornado), you comments would be very helpful.

1. first, i just want to confirm that sockjs-tornado by default sends heartbeat message 'h' to sockjs-client now, as shown here: https://github.com/mrjoes/sockjs-tornado/blob/master/sockjs/tornado/session.py#L257

2. to add support for heartbeats coming from sockjs-client (for now, this is implemented at the application level in my app), i just need to add a periodic callback in each connected SockJSConnection instance, for example:

from sockjs.tornado import SockJSConnection, SockJSRouter, periodic

class MyConnection (SockJSConnection):

    def on_open(this, info):

        ...

        this._heartbeat_timer = periodic.Callback(self._heartbeat, self._heartbeat_interval, tornado.io_loop)

        this._heartbeat_timer.start()

    

    def on_message(this, msg):

        if (msg == MyConnection.HEARTBEAT):

            this._missed_heartbeats = 0

        // other app logic

        ...

 

    def on_close(this):

        ...

        this._heartbeat_timer.stop()

        this._heartbeat_timer = None

    def _heartbeat(self):

        self._missed_heartbeats += 1

        if self._missed_heartbeats > 2:

            self.close()

In the code above, I am copying from the existing heartbeat code in sockjs-tornado to use periodic.Callback from sockjs-tornado instead of the usual tornado.ioloop.PeriodicCallback, is this OK? Also, in the callback, i directly reference tornado.io_loop, is this correct? Lastly, I notice that there is an existing method called delay_heartbeat in sockjs-tornado, just wonder what it is used for?

thanks,

Chris 

[Serge S. Koval]

Hi, 

 My comments are inline.

On Thu, Nov 15, 2012 at 7:39 PM, Chris <bhp...@gmail.com> wrote:

1. first, i just want to confirm that sockjs-tornado by default sends heartbeat message 'h' to sockjs-client now, as shown here: https://github.com/mrjoes/sockjs-tornado/blob/master/sockjs/tornado/session.py#L257

Yes, SockJS protocol requires heartbeats to be sent if connection is idling. delay_heartbeat() prevents sending heartbeat for another period.

 

Also, these heartbeats are only meant to keep idling connection alive and they won't help you to detect if connection died.

2. to add support for heartbeats coming from sockjs-client (for now, this is implemented at the application level in my app), i just need to add a periodic callback in each connected SockJSConnection instance, for example:

Code looks fine, except of the io_loop parameter to PeriodicCallback. Either remove it or pass real IOLoop instance. Also, make sure you define self._heartbeat_interval.

In the code above, I am copying from the existing heartbeat code in sockjs-tornado to use periodic.Callback from sockjs-tornado instead of the usual tornado.ioloop.PeriodicCallback, is this OK?

Does not matter - you won't need delay_heartbeat() functionality.

 

Lastly, I notice that there is an existing method called delay_heartbeat in sockjs-tornado, just wonder what it is used for?

To delay sending next heartbeat.

Serge. 

[Chris]

Code looks fine, except of the io_loop parameter to PeriodicCallback. Either remove it or pass real IOLoop instance. Also, make sure you define self._heartbeat_interval.

Thanks for clarifying the stuff for me. This IOLoop instance is where I am confused. I thought normally there is only one IOLoop instance in a tornado app, and I have seen people do things like:

def MyHandler(RequestHandler):

     def my_method(self):

          ...

          cb = tornado.ioloop.PeriodicCallback(...)

          ...

that's why I pass in tornado.ioloop in my code eariler, as I am not sure how I can reference this IOLoop reference in my app inside on_message method of sockjs-tornado. This is how I structure my sockjs-tornado app:

class Application(tornado.web.Application):

    def __init__(self):

        MyRouter = SockJSRouter(MyConnection, '/sockjs')

        handlers = [

            (r'/handle/([a-zA-Z0-9_]+$)', MyHandler)

        ] + MyRouter.urls

        settings = {

            'template_path': os.path.join(os.path.dirname(__file__), 'templates'),

            'static_path': os.path.join(os.path.dirname(__file__), 'static'),

            'debug': True

        }

        tornado.web.Application.__init__(self, *handlers, **settings)

class MyHandler(RequestHandler):

    def get(self):

        ...

class MyConnection(SockJSConnection):

    def on_open(self, info):

        ...

    def on_message(self, msg):

        ...

    def on_close(self):

        ... 

if __name__ == "__main__":

    tornado.options.parse_command_line()

    http_server = tornado.httpserver.HTTPServer(Application())

    http_server.listen(8080)

    tornado.ioloop.IOLoop.instance().start()