> On May 28, 2015, 4:22 a.m., abdelhakim deneche wrote: > > is the description found in JIRA 2903 relevant to this patch ?
DRILL-2967 is sufficient to fix what the JIRA requires. I am adding additional code like repititions to make the tests more robust. > On May 28, 2015, 4:22 a.m., abdelhakim deneche wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java, > > line 358 > > <https://reviews.apache.org/r/34690/diff/1/?file=972384#file972384line358> > > > > I know we should try to cleanup the code with each new patch we submit, > > but changing files just to clean them makes it a little harder to review > > the "real" changes involved in this patch. This change is not "just to clean". Not having this annotation can have unforeseen consequences e.g. if we remove the close() method from the interface, the compiler complains if the annotation is present. Additionally, any calls to the method within the class would also need to be removed. There was bug introduced in PojoRecordReader because of something like this. Instead of filing a JIRA, I made this change part of my patch. > On May 28, 2015, 4:22 a.m., abdelhakim deneche wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java, > > line 110 > > <https://reviews.apache.org/r/34690/diff/1/?file=972392#file972392line110> > > > > how doew junit handle timeouts when we repeat a test ? do we get > > timeouts if we increase the repeat count too much ?? The timeout is per iteration. > On May 28, 2015, 4:22 a.m., abdelhakim deneche wrote: > > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java, > > line 189 > > <https://reviews.apache.org/r/34690/diff/1/?file=972392#file972392line189> > > > > ZookeeperHelper javadoc already explains what happens when we pass > > "true" to it's constructor. We don't need to repeat the explanation here. I think we should repeat it. startSomeDrillbits() has a series of comments to explain how the cluster is being configured, and additional comments never hurt. - Sudheesh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34690/#review85504 ----------------------------------------------------------- On May 27, 2015, 12:52 a.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34690/ > ----------------------------------------------------------- > > (Updated May 27, 2015, 12:52 a.m.) > > > Review request for drill, abdelhakim deneche, Chris Westin, and Venki > Korukanti. > > > Bugs: DRILL-2903 > https://issues.apache.org/jira/browse/DRILL-2903 > > > Repository: drill-git > > > Description > ------- > > DRILL-2903: General improvements in TestDrillbitResilience > + Added RepeatTestRule to repeat tests > + Added ControlsInjectionBuilder to build controls in tests > + Added @Ignore and filed JIRAs for failing tests > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/util/RepeatTestRule.java > PRE-CREATION > common/src/main/java/org/apache/drill/common/util/TestTools.java 5be8d40 > common/src/test/java/org/apache/drill/test/DrillTest.java bbe014f > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java > 6176f77 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ProtobufLengthDecoder.java > 4f075d3 > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControlTunnel.java > 16b9b63 > > exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjectionImpl.java > 561d816 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlMessageHandler.java > 8ee7d38 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java > 5d07b49 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java > 71b77c6 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java > e5e0700 > > exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java > 696aed8 > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/ControlsInjectionBuilder.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/ControlsInjectionUtil.java > 346c6dd > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestCountDownLatchInjection.java > c98f54c > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java > e3558a1 > > exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestPauseInjection.java > ba29c58 > > Diff: https://reviews.apache.org/r/34690/diff/ > > > Testing > ------- > > Currently running unit tests. Need to run regression tests. > > > Thanks, > > Sudheesh Katkam > >
