> On 2012-02-21 06:40:54, Arvind Prabhakar wrote: > > Thanks for the patch Juhani. Can you please rebase it and update the > > review? It is not applying cleanly for MemoryChannel.java. > > > > Juhani Connolly wrote: > I just diffed patch-3 and patch-4(from my updated diff below) and they > appear to be identical. > Can you paste the output of the failed apply attempt? > > Juhani Connolly wrote: > just to be totally sure, I did a fresh svn checkout/patch and it was fine. > > 1032 mkdir flume > 1033 cd flume > 1034 svn checkout > https://svn.apache.org/repos/asf/incubator/flume/branches/flume-728/ > 1035 cd flume-728/ > 1037 patch -p1 < ~/workspace/flume/FLUME-936-4.patch > 1038 mvn test
Thanks Juhani. I probably did not apply the patch correctly or had some local changes at the time that may have caused conflict. Since I do not have the workspace now I cannot say for sure what the problem was. However, since the patch was committed, chances are there was no problem and it was a pilot error on my part. Thanks for making doubly sure of it though. - Arvind ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3704/#review5234 ----------------------------------------------------------- On 2012-02-21 06:51:00, Juhani Connolly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3704/ > ----------------------------------------------------------- > > (Updated 2012-02-21 06:51:00) > > > 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 > 6a17f06 > 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 > 3014368 > 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 > >
