FLUME-681: high msg rate syslogTcpThreadSource threads can lead to OOM

Review Request #1846 - Created June 24, 2011 and submitted

Jonathan Hsieh
old-flume
flume-681
flume
chetan, jon
Posted patch for Chetan.

Replaced unbounded queue with one that is bounded.


  1. 
      
  2. Could you make this configurable value, or file a jira to make it one?
    
    1. I guess this should go in the XML config, correct?
    2. xml is like a global setting
      adding an optional arg or kw arg to a build makes it more scoped.
      
      I think I'd prefer a kwarg/optional arg.
      
      For the short term, could you just file a follow-on work jira?  Adding the options in this patch would require unit tests and some more review steps.  It is easier to take the trivial bug-fixing patch as-is given current time constraints, and allow the follow-on work to happen later. 
    3. Sure, will fire a jira for it. I think it makes more sense to be in the xml config, though. It looks like the thrift source already has the same param in there - flume.thrift.queuesize - and there are a few other sources using the same sort of internal buffer:
      
      $ grep -r 'new LinkedBlockingQueue' * | grep handler | wc -l
      6
      
      I think it would make sense to do the same fix for all sources and combine the config into a single item. Also, the queue size is tied directly to the max heap size given to the JVM, so changing this value on the fly would not be a good idea. 
    4. I'm basically would like it to eventually be parameterizable, and don't have have a strong reason to like more than the other.  Either way seems reasonable, and aren't mutually exclusive. (hmm.. maybe both)...
  3. 
      
Review request changed

Change Summary:

Fixing typo and adding jira link.

Summary:

-FLUME-681: high msg rate syslogTcpThraedSource threads can lead to OOM
+FLUME-681: high msg rate syslogTcpThreadSource threads can lead to OOM

Bugs:

+flume-681
  1. lgtm.
  2. 
      
Loading...