Github user miguno commented on the pull request:

    https://github.com/apache/storm/pull/566#issuecomment-106795826
  
    > My reasoning is as follows:
    > 
    > * One has three option to deal with Netty's buffer filling up:
    >     1. Discard incoming new messages
    >     2. Block client thread until there is space (back pressure)
    >     3. Keep buffering up until OOME is thrown
    >
    > My guess is that the v0.9.4 code attempted to implement option (i), but 
actually the behavior is option (iii).
    
    The code's intention was actually (iii).  As you described back pressure 
(ii) was not picked because this will require a significant amount of work, 
which was thus out of scope for fixing STORM-329.  The reason (iii) was 
preferred over (i) was also as you described -- if (and only if) users have 
enabled acking = guaranteed message processing for a topology, they can prevent 
OOM errors from happening by setting 
[`topology.max.spout.pending`](https://github.com/apache/storm/blob/master/conf/defaults.yaml#L180).
    
    I'll have to look at your code in more detail before commenting.  I'll also 
ping @ptgoetz, @clockfly, @tedxia, and @revans2 who were involved in this 
significant patch (sorry for the spam, folks!).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to