netty/netty — pull request #2969 by fredericBregier (2014-10-05 18:22:00+00:00)
Title: Proposal to change the TrafficShapingHandlers for V3.9

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.

Without "softWritability" capability added to the channel, partial writability could be managed, using isWritable() and channelInterestChanged().
This allows for instance HttpChunkedInput to be partially 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 handles the channelInterestChanged(), filtering the events. This allows in some extent the compatibility with code using natively channelInterestChanged(). However, wihtout "softWritability" as proposed in 4.X, one should add a special management of those events in the final handler, as exposed in the example.

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.

The traffic shaping is more compatible with code as ChunkedWriteHandler, but up to a certain extent, in the sens that it does not prevent fully OOM since isWritable is only managed by the channel properties, not by adding a "softWritability". However, globally, it seems limiting natively even for ChunkedWriteHandler correctly the memory usage.

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.getChannel().isWritable() in conjunction of channelInterestChanged()
using a code as ChunkedFile to send the chunks according to the channelInterestChanged()
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.