netty/netty — pull request #3015 by fredericBregier (2014-10-16 10:17:27+00:00)
Title: V4.0 Final proposal to extend Channel to support "softWritability"

Motivation:
Several issues were shown by various ticket (#2900 #2956).

Issue #2900
When a huge amount of data are written, the current behavior of the
TrafficShaping handler is to limit the delay to 15s, whatever the delay
the previous write has. This is wrong, and when a huge amount of writes
are done in a short time, the traffic is not correctly shapened.

Moreover, there is a high risk of OOM if one is not using in his/her own
handler for instance ChannelFuture.addListener() to handle the write
bufferisation in the TrafficShapingHandler.

As for autoRead (setAutoRead), I think it could be useful to allow
someone to set a "softWritability" property to the channel. Doing this,
this will allow some handlers to "soft" managed writability directly, as
for reading, without the need to create another Attribute neither
another event, but using current isWritable() and
channelWritabilityChanged().
This allows for instance HttpChunkedInput to be fully compatible.

The "bandwidth" compute on write is only on "acquired" write orders, not
on "real" write orders, which is wrong from statistic point of view.

Issue #2956
When using GlobalTrafficShaping, every write (and read) are
synchronized, thus leading to a drop of performance.
ChannelTrafficShaping is not touched by this issue since synchronzed is
then correct (handler is per channel, so the synchronized).

Modifications:
The current write delay computation takes into account the previous
write delay and time to check is the 15s delay (maxTime) is really
exceeded or not (using last scheduled write time). The algorithm is
simplified and in the same time more accurate.

This proposal adds 2 new methods to Channel (isSoftWritable() and
softWritable) in order to manage this value, and to change isWritable()
by taking into account this new softWritable property.

When the real write occurs, the statistics are update accordingly on a
new attribute (getRealWriteThroughput()).

To limit the synchronisations, all synchronized on
GlobalTrafficShapingHandler on submitWrite were removed. They are
replaced with a lock per channel (since synchronization is still needed
to prevent unordered write per channel), as in the sendAllValid method
for the very same reason.
Also all synchronized on TrafficCounter on read/writeTimeToWait() are
removed as they are unnecessary since already locked before by the
caller.
Still the creation and remove operations on lock per channel (PerChannel
object) are synchronized to prevent concurrency issue on this critical
part, but then limited.

Result:
The traffic shaping is more stable, even with a huge number of writes in
short time by taking into consideration last scheduled write time.

When changing the current proposal implementation of
TrafficShapingHandler using softWritability management and default
fireChannelWritabilityChanged, it works like a charm and does not need
those extra Attribute/user event. Could be used anywhere else too where
a "bufferisation" is made in front of the channel default bufferisation.

The statistics are more valuable (asked write vs real write).

Various examples are shown in one added example (example/traffic), and
in particular how a user handler might managed the write to prevent OOM:

using the ChannelFuture.addListener(new GenericFutureListener>() one by
one, or by set of writes
using the ctx.channel().isWritable() and channelWritabilityChanged()
using a code as HttpChunkedInput to send the chunks according to the
channelWritabilityChanged() (softWritability is needed)
using no specific check, but with the risk of OOM (no control on the
bufferisation made by the TSH)

The Global TrafficShapingHandler should now have less "global"
synchronization, hoping to the minimum, but still per Channel as needed.