Thx Brock. I will make the changes as soon as I get home. On Sep 7, 2012 10:59 AM, "Brock Noland" <[email protected]> wrote:
> This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6949/ > > Nice work! A few minor issues below to resolve and then I will commit this! > > > > flume-ng-core/src/main/java/org/apache/flume/source/StressSource.java<https://reviews.apache.org/r/6949/diff/1/?file=150546#file150546line47> > (Diff > revision 1) > > 46 > > private CounterGroup counterGroup; > > 47 > > CounterGroup counterGroup; > > Let's keep this private and then use the fast reflection api to get the > reference in the test. > > > > flume-ng-core/src/main/java/org/apache/flume/source/StressSource.java<https://reviews.apache.org/r/6949/diff/1/?file=150546#file150546line54> > (Diff > revision 1) > > 53 > > Event event; > > We should keep these as private variables. > > > > flume-ng-core/src/main/java/org/apache/flume/source/StressSource.java<https://reviews.apache.org/r/6949/diff/1/?file=150546#file150546line92> > (Diff > revision 1) > > private void prepEventData(int bufferSize) { > > 91 > > Arrays.fill(buffer, (byte)(Math.random() * Byte.MAX_VALUE)); > > We don't copy the buffer in EventBuilder. As such there is only one > "buffer" and we are updating all events with this fill. > > I'd prefer to keep those semantics as opposed to creating a new buffer for > each event. > > > > flume-ng-core/src/test/java/org/apache/flume/source/TestStressSource.java<https://reviews.apache.org/r/6949/diff/1/?file=150547#file150547line42> > (Diff > revision 1) > > public class TestStressSource { > > 37 > > public class TestStressSource { > > 42 > > public class TestStressSource extends TestCase{ > > This is not needed. > > > > flume-ng-core/src/test/java/org/apache/flume/source/TestStressSource.java<https://reviews.apache.org/r/6949/diff/1/?file=150547#file150547line58> > (Diff > revision 1) > > public class TestStressSource { > > public class TestStressSource extends TestCase{ > > 58 > > return source.eventBatchList; > > Instead of referencing the member variable, let's keep them private and > then use the fast reflection api (like the method directly above). > > > > flume-ng-core/src/test/java/org/apache/flume/source/TestStressSource.java<https://reviews.apache.org/r/6949/diff/1/?file=150547#file150547line76> > (Diff > revision 1) > > public void testMaxTotalEvents() throws InterruptedException, > > 76 > > public void testBatchEvents() throws InterruptedException, > > Add the @Test annotation above this method and the extends TestCase is not > required. > > > - Brock > > On September 7th, 2012, 1:06 a.m., Ted Malaska wrote: > Review request for Flume. > By Ted Malaska. > > *Updated Sept. 7, 2012, 1:06 a.m.* > Description > > Adding batchSize to StressSource along with junit to test it. > > Important note: the total number of events getting sent will equals > maxTotalEvents, so if maxTotalEvents is not devisable by batchSize then the > last batch will contain only the remainder of events needed to reach > maxTotalEvents. > > *Bugs: * FLUME-1536 <https://issues.apache.org/jira/browse/FLUME-1536> > Diffs > > - flume-ng-core/src/main/java/org/apache/flume/source/StressSource.java > (5b73910) > - flume-ng-core/src/test/java/org/apache/flume/source/TestStressSource.java > (4ec16c7) > > View Diff <https://reviews.apache.org/r/6949/diff/> >
