Inquiry: Timeouts and Comet

[Nicholas Whitehead]

Hi guys;

I was noodling with Deft, wanting to create a fast, small Comet server, but there's a few things I am not clear on.

Here's my starting noodle:

I created a Request handler to generate random numbers every 3 seconds.

       @Asynchronous
       public void get(HttpRequest request, final HttpResponse response) {
           startRandomNumberThread(new AsyncResult<String>() {
            @Override
            public void onFailure(Throwable caught) {
                response.finish();
            }
            @Override
            public void onSuccess(String result) {
                response.write(result).flush();
            }
           });
       }


The random number thread writes the number (as a string) to the AsyncResult every 3 seconds. I tested the service using curl like this:

 curl --keepalive-time 5 http://localhost:8787/random
-8384884039211184659
-5844311697439096390

So it actually works  (until it times out). So my questions are:

1. In general,  it seems the best approach for Deft is to define all handlers as @Asynchronous and delegate the actual tasks to thread pools. More or less correct ?
2. How do I finagle things so that the TimeoutManager does not kill my "comet" sessions ?
3. Conversely, how do I detect when the client has disconnected so I can stop the thread and not keep writing into the DynamicByteBuffer ?

At some point, I would replace the new thread with a scheduled thread pool executor and hand off all the work to it, but that's the easy part. It's the items above that have confounded me.

If this seems worthwhile, I can create a documented and clean version for a Deft example collection.

Cheers.

//Nicholas

[Roger Schildmeijer]

Hi Nicholas,

First a working example: https://gist.github.com/811836

Answers:

1, No. Delegate to thread pools is a no no. Deft is single threaded and non blocking. All "long term" operations must be executed asynchronously ("callback based"). Only annotate a RequestHandler with @Asynchronous if its doing any asynchronous operations.

2, The best way to fix this on the 0.2.0 release is to change the KEEP_ALIVE_TIMEOUT (HttpServerDescriptor.KEEP_ALIVE_TIMEOUT, 30s default) to a high value. 

Actually you found a bug here, even if there is IO activity on the socket within the timeout period (30s) the connection will be closed. (Because there is no explicit call to TimeoutManager.touch(SelectableChannel)). Actually we should consider (atleast investigate the impact) to remove the connection keep alive timeout. E.g I dont think facebook/tornado has keep alive timeouts.

3, Good point, might be an issue that needs to be addressed.

// Roger

[Roger Schildmeijer]

Missed some points.

A) The example will be much more elegant and shorter when
PeriodCallbacks are available in Deft (issue #90)
B) The "working example", pasted in the previous mail, is running
against a 0.3.0-SNAPSHOT. It will not work against 0.2.0 because of
issue #89, https://github.com/rschildmeijer/deft/issues/closed#issue/89

> >  curl --keepalive-time 5http://localhost:8787/random

[Nicholas Whitehead]

Thanks for the response Roger;

I am still a little hazy on #1. If the I/O-Loop thread receives a request, calls a [@Asynchronous] RequestHandler which in turn creates a "Task" object equipped with an AsyncResult , queues the task for execution and then returns, as far as the I/O-Loop thread is concerned, this would qualify as an asynchronous task, no ? (In this scenario, a thread pool thread grabs the task off the queue, does something useful with it and writes the result to the AsyncResult). It seemed to me that this is more or less what the AsyncDbApi is fictitiously doing.

I noticed the touch command, but was not sure how it was intended to be used.

//Nicholas

[Roger Schildmeijer]

On 4 feb 2011, at 23.29em, Nicholas Whitehead wrote:

Thanks for the response Roger;

I am still a little hazy on #1. If the I/O-Loop thread receives a request, calls a [@Asynchronous] RequestHandler which in turn creates a "Task" object equipped with an AsyncResult , queues the task for execution and then returns, as far as the I/O-Loop thread is concerned, this would qualify as an asynchronous task, no ?

This sure is asynchronous, but unfortunately not the way to do async coding in single threaded event-driven systems like Deft.

(In this scenario, a thread pool thread grabs the task off the queue, does something useful with it and writes the result to the AsyncResult).

This is not a good idea because the behaviour is not deterministic if another thread than the ioloop thread is accessing e.g HttpResponse. 

It seemed to me that this is more or less what the AsyncDbApi is fictitiously doing.

AsyncDbApi is obsolete and should be removed. Sorry for that. 

A better example to look at is org.deftserver.example.kv.

The important thing in that example is that every io operation is controlled by the ioloop.

Im currently working on #97 (Simple asynchronous http client), and hopefully things will be more clear when this is issue is resolved.

I noticed the touch command, but was not sure how it was intended to be used.

Intended to be used internally to prolong keep alive connection timeouts.

//Nicholas

// Roger

[Roger Schildmeijer]

Been think about your thread pool, task, queue approach. We might need
a way support this use case. It reminds me of the documentation about
the add_callback method in tornados ioloop:

"""Calls the given callback on the next I/O loop iteration.

It is safe to call this method from any thread at any time.
Note that this is the *only* method in IOLoop that makes this
guarantee; all other interaction with the IOLoop must be done
from that IOLoop's thread. add_callback() may be used to transfer
control from other threads to the IOLoop's thread.
"""

We could (and probably should) offer this possibility. To guarantee
thread safety we must change (maybe some more synchronization stuff)
the collection used in JDCM (JMXDebuggableCallbackManager). Because we
dont know which thread that will be adding the callback.

Thougts comments?

// Roger

> >>  curl --keepalive-time 5http://localhost:8787/random

[Roger Schildmeijer]

Good documentation (most apply to Deft also) about the subject on the tornado wiki:

https://github.com/facebook/tornado/wiki/Threading-and-concurrency

Thanks Ben Darnell.

/ Roger

[Nicholas Whitehead]

It sounds good to me. I am still going through the examples you outlined, but synchronising that collection should be straight-forward enough.

[Roger Schildmeijer]

Cool. Just let me know if some parts of the example doesn't make sense.

Roger