-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3704/
-----------------------------------------------------------

(Updated 2012-02-02 10:23:16.661600)


Review request for Flume.


Changes
-------

One more diff because I had only been running the ng-core tests. Some of the 
other tests also fail without adjusting the memory channel configuration(I just 
made the defaults larger rathar than adjusting many different tests)

Additionally, when running the tests via mvn as opposed to running them one at 
a time through my IDE, testConcurrentSinksAndSources fails with mismatched put 
and take counts. I cannot recreate this regardless of how many times I run the 
test on its own so I'm pretty sure that it is because of mvn concurrently 
running multiple tests at a time which are then interfering with one another, 
though if anyone has any other ideas I'm all ears.


Summary
-------

This is an initial go at fixing the threading issues with memory channel. 

It uses the preliminary work on FLUME-935 and I have included the code from 
that.

The tagging of the events became unnecessary so I dropped that. One thing that 
concerns me slightly is how to deal with not having enough space in the queue 
to rollback failed takes. One method would be to keep a minimum buffer of 
transactionCapacity. Another would be to implement the queue of queues as 
suggested in FLUME-889

Anyway, just putting up this early version to see what people think


This addresses bug FLUME-936.
    https://issues.apache.org/jira/browse/FLUME-936


Diffs (updated)
-----

  
flume-ng-core/src/main/java/org/apache/flume/channel/BasicChannelSemantics.java 
PRE-CREATION 
  
flume-ng-core/src/main/java/org/apache/flume/channel/BasicTransactionSemantics.java
 PRE-CREATION 
  flume-ng-core/src/main/java/org/apache/flume/channel/ChannelUtils.java 
PRE-CREATION 
  flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java 
d379b64 
  flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java 
b44030e 
  
flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelConcurrency.java
 PRE-CREATION 
  
flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelTransaction.java
 d18045b 
  flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java 
46e42e3 
  flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 
9e465e1 

Diff: https://reviews.apache.org/r/3704/diff


Testing
-------

The original tests pass, though I had to take out the state checks because of 
the changes to semantics from the flume-935 code. I also had to add a 
transaction.close statement where semantics were not properly being followed
I have to retrofit my new concurrency test since without the tagged events it 
cannot fail without checking that the content is correct. I'll put that up 
asap, just wanted to get some eyes on this before I head out.


Thanks,

Juhani

Reply via email to