*TimeOutHandlers

[Christian Migowski]

Hi,

two more (minor) bugs to squash:
IdleStateHandler line 240 should be
channelIdle(ctx, IdleState.WRITER_IDLE, lastWriteTime);
IdleStateHandler line 274 should be
channelIdle(ctx, IdleState.ALL_IDLE, lastIoTime);
(in both the last parameter is wrong)

BTW, I am no Java guru, but is there a special point in doing e.g.
long lastWriteTime = IdleStateHandler.this.lastWriteTime;
in the inner classes instead of just accessing it directly (I always
seek to extend my Java knowledge).


>> 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)
>
> As an alternative, I added constructors which doesn't require
> ticksPerWheel, and therefore a user can choose the precision of the
> timer pretty easily.  (e.g. new HashedWheelTimer(1, SECONDS))  The
> default ticksPerWheel is 512, which should work fine in general.  BTW
> I think most people should be fine with using the default constructor,
> which has the sensible default.

ok

>> The creation of the timer could be even integrated in the constructor
>> of the timeout handlers.
>
> This can be somewhat confusing.  For example, a user can create
> thousands of HashedWheelTimer threads if he or she doesn't understand
> what is created inside the timeout / idle state handlers.  So, I'd
> like to keep it as it is.  Instead, I'm going to implement a shared
> resource explosion detector, which gives a user warning when expensive
> resources are created too much (e.g. 1000 HashedWheelTimers or 1000
> ChannelFactories)

agree, you are right

>>  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)
>
> Fixed.  BTW you don't need to add L, it will be automatically converted. ;)

but tools like FindBugs or PMD which do even more nitpicking than me
complain when you omit the L ;-)

>> - 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)
>
> I agree that this indeed is annoying.  Let me come up with the best
> string format.  Any suggestion would be appreciated.  I think the
> channel UUID needs to be included anyway because it will be very
> helpful for analyzing a large log file.

yes, the channel info should be included, it is useful. I think it is
sufficient to just change positions, put the getChannel().toString()
at the end of the string (after all this is just for debugging and
doesn't need to be thought about in great detail)

>> 4) personal wish
>> any chance that the IdleStateEvent gets promoted to a "proper"
>> UpstreamEvent (i.e. there will be a channelIdle() method in
>> ChannelUpstreamHandler)?
>
> I think I can create IdleStateAwareChannelHandler in the
> org.jboss.netty.handler.timeout package, which extends
> SimpleChannelHandler instead.  I think this is the beauty of Netty's
> flexible event model and this should work for you. ;)

It's extensibility is indeed quite cool.
Thanks a lot,
best regards,

christian