*TimeOutHandlers

Thu Feb 12 16:06:14 EST 2009

Lets hope we can turn the page to the next one soon, just a few more
comments on this one:

1) obvious bug (NPE, will always happen (at least in rev 869)): add
allIdleTimeoutTask = new AllIdleTimeoutTask(ctx);
after line 146 in IdleStateHandler

2) the HashedWheelTimer should get some additional userfriendly
constructors, because nobody wants to know what tickDuration and
ticksPerWheel are good for. >;-D
I am thinking of something like
HashedWheelTimer(HashedWheelTimer.QUITE_EXACT) which resembles to
HashedWheelTimer(1, MILLISECONDS,100) and
HashedWheelTimer(HashedWheelTimer.RESOURCE_EFFICIENT) which could be
like HashedWheelTimer(1, SECONDS,10)

The creation of the timer could be even integrated in the constructor
of the timeout handlers.

 3) upsetting nit-picking :-)
- now the default timeunit has changed to seconds in the handler
constructors, the parameters could be int instead of long again
(making the necessary code smaller by saving three "L" characters :D)
- IMHO the toString methods of the various ChannelEvents should print
what they are first, then the Channel information  (I need to scroll
to see what was just logged)

4) personal wish
any chance that the IdleStateEvent gets promoted to a "proper"
UpstreamEvent (i.e. there will be a channelIdle() method in
ChannelUpstreamHandler)?

otherwise: good job!

regards,
christian

On Thu, Feb 12, 2009 at 4:43 PM, Trustin Lee <trustin at gleamynode.net> wrote:
> Glad that we are on the same page again. :-D
>
> I have made major changes as listed below:
>
>  * Renamed: IdlenessHandler -> IdleStateHandler
>  * Renamed: IdlenessEvent -> IdleStateEvent
>  * Added: IdleState enum which is consisted of READER_IDLE,
> WRITER_IDLE, ALL_IDLE
>  * Added: allIdleTime (bothIdleTime in MINA term)
>  * Changed: the default time unit (milliseconds -> seconds, which
> makes more sense)
>  * Added: callback methods in IdleStateHandler, ReadTimeoutHandler,
> and WriteTimeoutHandler so that you can define a custom behavior by
> extending them.  The default behavior is to fire IdleStateEvent, to
> raise ReadTimeoutException, and to raise WriteTimeoutException
> respectively.
>
> There might be other subtle changes I didn't list here by mistake, so
> please feel free to give me feed back and share your findings.  I
> think idle state detection and timeout handling is pretty much done
> now though, all thanks to both of you.  :)
>
> — Trustin Lee, http://gleamynode.net/
>
>
>
> On Wed, Feb 11, 2009 at 7:34 PM, Christian Migowski
> <chrismfwrd at gmail.com> wrote:
>> Trustin,
>>
>> sorry, somehow (wrote the mail before I had coffee) I misunderstood
>> your comments and thought you thought (oh boy) it would be useful to
>> get the timeout exceptions even if the channel is disconnected.
>>
>> We were always on the same page, I was just confused - thanks for clarification!
>>
>> I also did a quick test of the IdlenessHandler and it works like I was
>> thinking first it should (i.e. same as sessionIdle() in Mina), thanks
>> a lot! :-)
>> Since both both *TimeoutTasks report the write and the read idleness
>> (was this intentional? The *TimeoutTasks can access the last*Time
>> properties of the IdlenessHandler), I think you could remove one
>> TimeoutTask.
>>
>> What I mean is that is Handler:
>>        pipeline.addLast("idlehandler", new IdlenessHandler(timer,10000L,5000L));
>>
>> with this extension to my TestHandler:
>>    public void handleUpstream(ChannelHandlerContext ctx, ChannelEvent
>> e) throws Exception {
>>        if (e instanceof IdlenessEvent) {
>>            System.out.println(new
>> SimpleDateFormat("HH:mm:ss.SSS").format(new Date())+"idleness event:
>> "+e);
>>        }
>>        super.handleUpstream(ctx, e);
>>    }
>>
>> produces this output:
>> 1:15:15.508idleness event: NioAcceptedSocketChannel(id:
>> 64d52f74-011f-1000-bb5d-0139b1fabbcf, /127.0.0.1:9873 =>
>> /127.0.0.1:12345) - (IDLE: RW)
>> 11:15:20.508idleness event: NioAcceptedSocketChannel(id:
>> 64d52f74-011f-1000-bb5d-0139b1fabbcf, /127.0.0.1:9873 =>
>> /127.0.0.1:12345) - (IDLE: RW)
>> 11:15:20.523idleness event: NioAcceptedSocketChannel(id:
>> 64d52f74-011f-1000-bb5d-0139b1fabbcf, /127.0.0.1:9873 =>
>> /127.0.0.1:12345) - (IDLE: RW)
>> 11:15:25.539idleness event: NioAcceptedSocketChannel(id:
>> 64d52f74-011f-1000-bb5d-0139b1fabbcf, /127.0.0.1:9873 =>
>> /127.0.0.1:12345) - (IDLE: RW)
>> 11:15:30.523idleness event: NioAcceptedSocketChannel(id:
>> 64d52f74-011f-1000-bb5d-0139b1fabbcf, /127.0.0.1:9873 =>
>> /127.0.0.1:12345) - (IDLE: RW)
>>
>>
>>
>> regards,
>> christian
>>
>> On Wed, Feb 11, 2009 at 10:42 AM, Trustin Lee <trustin at gleamynode.net> wrote:
>>> On Wed, Feb 11, 2009 at 5:12 PM, Christian Migowski
>>> <chrismfwrd at gmail.com> wrote:
>>>> On Wed, Feb 11, 2009 at 6:01 AM, Trustin Lee <trustin at gleamynode.net> wrote:
>>>>> I was able to reproduce the problem Dave and Christian reported and
>>>>> have just checked in the fix.
>>>>>
>>>>> I'm still not sure ChannelReadTimeoutException should be fired only
>>>>> once while a channel is connected.  I think it's just fine to raise
>>>>> the exception periodically, and it's sometimes useful.
>>>>
>>>> Really? Maybe thats where an architectural overview of Netty and its
>>>> design goals would have come in handy, are Netty channels "reusable"?
>>>> I was under the impression that you'll get a "new" channel when the
>>>> client connects again to the server or if you do a new connect() with
>>>> a client.
>>>
>>> I'm not sure we are on the same page.  Let me rephrase what I meant to
>>> avoid misunderstanding:
>>>
>>> 1) ReadTimeoutException should not be raised when a Channel is disconnected.
>>> 2) ReadTimeoutException should be raised periodically while a Channel
>>> is connected.
>>>
>>>> Could you outline a little bit why a Channel should raise the
>>>> ReadTimeoutException when it predictably will raise that exception
>>>> because there is nothing to read (channel disconnected)? You _can_
>>>> detect a disconnection with channelDisconnected(...), right?
>>>
>>> Yes, we can detect a disconnection with a channelDisconnected event
>>> and ReadTimeoutException should not be raised unless a Channel is
>>> connected.
>>>
>>> BTW, I have checked in IdlenessHandler which might fulfill your needs,
>>> but it seems like it's not being synchronized to the anonymous SVN
>>> repository at this moment.  Let me paste it for your convenience:
>>>
>>>  * IdlenessHandler - http://pastebin.com/m66e1e937
>>>  * IdlenessEvent - http://pastebin.com/m4bc7d5c5
>>>  * DefaultIdlenessEvent - http://pastebin.com/m6ea6b5b
>>>
>>> Once the synchronization between the committable repository and the
>>> read-only repository is recovered, they should appear here:
>>>
>>>  * http://fisheye.jboss.org/browse/Netty/trunk/src/main/java/org/jboss/netty/handler/timeout
>>>
>>> Thanks for the feed back,
>>>
>>> — Trustin Lee, http://gleamynode.net/
>>>
>>> _______________________________________________
>>> netty-users mailing list
>>> netty-users at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/netty-users
>>>
>>
>> _______________________________________________
>> netty-users mailing list
>> netty-users at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/netty-users
>>
>
> _______________________________________________
> netty-users mailing list
> netty-users at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-users
>