FLUME-504: Update to Thrift 0.6

Review Request #1581 - Created Feb. 14, 2011 and submitted

Bruce Mitchener
old-flume
FLUME-504
flume
jon
Upgrades to Thrift 0.6.
Tests seem to pass.

Actually one fails ... I thought it was one of my later patches, but it was this one.
  1. Here's the result of a test run that I get:
    
    Test Result (3 failures / -1)
    com.cloudera.flume.agent.TestAgentFailChainSink.testConfirmBEChain
    com.cloudera.flume.agent.TestAgentFailChainSink.testConfirmDFOChain
    com.cloudera.flume.handlers.thrift.TestThriftEventAdaptor.testNullBody
    1. If you can drop by IRC ... I'd love to have some help on the first 2. The 3rd works for me.
    2. The first 2 are non-deterministic tests ... those failures are meaningless.
      
      The third one is because you didn't have the newly generated code. Passes fine with that.
      
    3. if they are non-deterministic, can they be changed them to be deterministic?  I think I really wanted to capture the fact that some events are lost and that eventually some are recovered, or eventually it starts accepting data again (these tests were because of a bugfix where retries just weren't happening for some reason).
      
      I haven't conformed, but I buy the reason for the last test not passing for me.
      
    4. Can you take a look and confirm that they are non-deterministic? I spent a couple of hours sorting through this yesterday as I really need this patch to land (along with another) so that I can land the real meat of my work.
    5. I have a hacked version that makes test pass -- NOTE I didn't really think about all the numbers I changed.  There was a different  subtle issue (the need to instantiate new events) that I've pushed to my personal repo for you to look at.  (github/jmhsieh/flume-504-hack).  
      
      check out this link: https://github.com/jmhsieh/flume/blob/flume-504-hack/src/javatest/com/cloudera/flume/agent/TestAgentFailChainSink.java
    6. I'd missed this ... I'm running a build with that now, if it passes, I'll update the diff here and would like to land it ASAP along with FLUME-521.
  2. 
      
Review request changed
  1. I'm cool with this if it passes tests.  Please check in the auto-generated code as a separate follow on commit.  
    
    Thanks, Jon.
  2. 
      
Loading...