FLUME-620: Avro/Thrift event truncation for events greater than 32kb

Review Request #1816 - Created June 15, 2011 and submitted

Jonathan Hsieh
old-flume
flume
arvind, esammer
Avro and thrift rpcs can get data from external sources that may not respect the max body length setting.  Previously flume used an adaptor pattern to wrap avro and thrift event objects.  Part of the problem with the previous approach is that event bodies that were too large could "sneak" in and cause flume to enter an endless error loop, essentially getting stuck. 

This now does a conversion instead and offers two options - dropping the event, or truncating it and passing it in.
tests pass. 
  1. There's a lot of debt build up here. We shouldn't leave trash during refactoring; just rename the classes. I think there's really poor class coupling here (logical node context used just for construction of a testing object is gross). Ship it if you want, but I think it's unfortunate; we really need to stop doing this kind of stuff. The code is already difficult to maintain.
    1. I've addressed most of your comments.  Added a jira to split off LogicalNodeContext.testingContext()
  2. This should get renamed. It's no longer than adaptor.
    1. renamed to AvroEventConvertUtil, and given private constructor to make it a util class.
  3. Looks like this should be private.
    1. I don't completely agree, but have made it private.
  4. Probably makes sense to make this private.
  5. nit: Why truncates rather than truncate?
    1. I'll make it truncate.  It makes it more consistent with other kw args.
  6. Rename comment, same as for AvroEventAdaptor.
  7. My comments about making these private in the avro class go here as well.
  8. Same naming nit as with AvroEventSource. Truncate is a command to the source, not an inherent attribute of the source so 'truncate' is more appropriate.
  9. naming nit: We've deviated from truncates to truncated here. We should at least be consistent. Again, the Source isn't "truncated," the events may be. 'truncate' or 'shouldTruncate' is more appropriate.
    1. sure, made other thrift and avro 'shouldTruncate'
  10. This is gross. We shouldn't have test utilities in the main code base and then use them in unrelated code.
    1. creating new jira to remove this construct
  11. This is pretty unreliable / not always true. Smells like a flaky test in the making.
    1. do you have a suggestion to improve this?
  12. 
      
Loading...