----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3704/#review5217 -----------------------------------------------------------
Juhani - can you please confirm if this change is the latest? I see this diff is named FLUME-936-2.patch while the one attached to the jira is FLUME-936-3.patch. Should I go ahead and review this or are you planning to update this diff? - Arvind On 2012-02-08 06:47:33, Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3704/ > ----------------------------------------------------------- > > (Updated 2012-02-08 06:47:33) > > > Review request for Flume. > > > 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 > ----- > > 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 > >
