> On Nov. 6, 2013, 2:49 p.m., Sean Busbey wrote: > > src/server/src/test/java/org/apache/accumulo/server/util/InitializeTest.java, > > line 31 > > <https://reviews.apache.org/r/15279/diff/1/?file=379598#file379598line31> > > > > Include a note that this test class is not threadsafe. > > > > The default poms and build scripts don't run parallel unit tests. But I > > know the build speed is something we care about, so we'll get there > > eventually. Best to have a warning for future maintainers. How much detail > > to include is up to you. > > > > It won't work with JUnit 4.7+'s parallel, since they're in the same JVM: > > > > > > http://maven.apache.org/surefire/maven-surefire-plugin/examples/junit.html#Running_tests_in_parallel > > > > It will work with Surefire's fork and reuseForks=false, since that's a > > JVM per class > > > > > > http://maven.apache.org/surefire/maven-surefire-plugin/examples/fork-options-and-parallel-execution.html#Forked_Test_Execution > > Bill Havanki wrote: > Will do. > > Bill Havanki wrote: > Another thought: we could start to introduce the JCIP annotations to > indicate thread safety (or lack of it) in a better way. I'll float the idea > to the dev list. > > Sean Busbey wrote: > Sure. Don't let the wider refactor that would be stop you from finishing > this issue though. :) > > I think starting to use JCIP annotations should be its own ticket. > > John Vines wrote: > 1.4.5 and 1.5.1 are bugfix releases, and I'm really only concerns about > fixing the ticket on hand. I'm really iffy to even qualify it as a something > that should be fixed in those releases as this really is an improvement, not > a bug. But it seems useful and lightweight, so fixing the ticket at hand is > fine. But introducing a whole bunch of other things in 1.4.x with it I'm > going to have to vote against. > > Sean Busbey wrote: > That's a major reason I want JCIP annotations in their own ticket. I > presume Bill was talking about using them primarily in the current major > version, but all of this is discussion unrelated to the task at hand. > > The current issue just needs a comment block so that 1.4, 1.5, and 1.6 > have a suitable warning.
My intention is not to try introducing JCIP with this ticket. I concur with Sean, it's its own ticket. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15279/#review28290 ----------------------------------------------------------- On Nov. 6, 2013, 2:02 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15279/ > ----------------------------------------------------------- > > (Updated Nov. 6, 2013, 2:02 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-1556 > https://issues.apache.org/jira/browse/ACCUMULO-1556 > > > Repository: accumulo > > > Description > ------- > > The Initialize class now generates clearer error messages if an initialized > instance is discovered. The messages vary depending on whether > instance.dfs.uri is used. > > Note that to facilitate unit testing, the verification logic in > Initialize.doInit() was refactored into a checkInit() method. > > > Diffs > ----- > > pom.xml 9ed2fdf1c7a1f8831667b27bfaa307fbe2467fe8 > src/server/pom.xml 6421bc69cda5116f9716d30adc3d510baa3bb7d6 > src/server/src/main/java/org/apache/accumulo/server/util/Initialize.java > 51576fcc8ff8ffcd65a78bfeaaaf51036360a6bc > > src/server/src/test/java/org/apache/accumulo/server/util/InitializeTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/15279/diff/ > > > Testing > ------- > > Ran initialization with patch changes on 1.4.3 cluster under CDH 4.3. Tested > successful initialization and correct emission of error messages when > instance.dfs.uri was used and was not used. Also, implemented unit tests for > altered code. > > > Thanks, > > Bill Havanki > >
