Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo, mail # dev - Review Request 15279: ACCUMULO-1556: Clarify initialization error messages for pre-initialized filesystem


Copy link to this message
-
Re: Review Request 15279: ACCUMULO-1556: Clarify initialization error messages for pre-initialized filesystem
Sean Busbey 2013-11-06, 20:23


> On Nov. 6, 2013, 7:49 p.m., Sean Busbey wrote:
> > src/server/src/main/java/org/apache/accumulo/server/util/Initialize.java, lines 23-25
> > <https://reviews.apache.org/r/15279/diff/1/?file=379597#file379597line23>
> >
> >     please don't include formatting fixes unrelatd to the change.
>
> Bill Havanki wrote:
>     The Eclipse code formatter insisted, and I'm told we should use it.

Break into another issue/patch then please.
> On Nov. 6, 2013, 7:49 p.m., Sean Busbey wrote:
> > src/server/src/main/java/org/apache/accumulo/server/util/Initialize.java, lines 71-72
> > <https://reviews.apache.org/r/15279/diff/1/?file=379597#file379597line71>
> >
> >     please don't include formatting fixes unrelated to the issue.
>
> Bill Havanki wrote:
>     The Eclipse code formatter insisted, and I'm told we should use it.

Break into another issue/patch then please.
> On Nov. 6, 2013, 7:49 p.m., Sean Busbey wrote:
> > src/server/src/main/java/org/apache/accumulo/server/util/Initialize.java, lines 179-181
> > <https://reviews.apache.org/r/15279/diff/1/?file=379597#file379597line179>
> >
> >     Does this buy us much beyond just letting the original IOException propagate without wrapping?
>
> Bill Havanki wrote:
>     Eh, not too much, but it states why the original IOException (upon trying to look in HDFS) happened.

Fair enough.
> On Nov. 6, 2013, 7: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.

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.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15279/#review28290
-----------------------------------------------------------
On Nov. 6, 2013, 7: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, 7: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