Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============7471909345136893678=="
MIME-Version: 1.0
Subject: Re: Review Request 30382: ACCUMULO-3514 Use auto service for start
From: "Christopher Tubbs" <ctubbsii-rb@apache.org>
To: "Christopher Tubbs" <ctubbsii-rb@apache.org>,
 "Josh Elser" <josh.elser@gmail.com>, "accumulo" <dev@accumulo.apache.org>,
 "Sean Busbey" <sean@manvsbeard.com>
Date: Sat, 31 Jan 2015 07:01:32 -0000
Message-ID: <20150131070132.1309.30310@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Christopher Tubbs" <noreply@reviews.apache.org>
References: <20150130160006.4427.78112@reviews.apache.org>
In-Reply-To: <20150130160006.4427.78112@reviews.apache.org>
Reply-To: "Christopher Tubbs" <ctubbsii-rb@apache.org>

--===============7471909345136893678==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Jan. 30, 2015, 11 a.m., Josh Elser wrote:
> > start/src/main/java/org/apache/accumulo/start/Main.java, line 49
> > <https://reviews.apache.org/r/30382/diff/2/?file=841134#file841134line49>
> >
> >     Let's make sure this gets answered rather than commit a question to the codebase (I don't know why it's needed either).
> 
> Christopher Tubbs wrote:
>     Agreed.
> 
> Christopher Tubbs wrote:
>     I checked with Dave Marion, and he seemed to think it wasn't needed. And, neither do I (hence my initial interrogative comment). So, I'm deleting it.
> 
> Sean Busbey wrote:
>     Setting the context classloader here to the one from Accumulo will impact how JNDI resolves classes before it is set again. (I don't know if anyone relies on this behavior)

I thought Keith had looked into this a bit, and suggested that it would only affect it if it was set before the thread started. Otherwise, it would only matter if some code was calling Main.main() directly and then retrieving the current thread's context class loader (which does not seem like a reasonable thing to do). All the classes we load when we let Java execute this as it's main class are loaded using the classloaders we set up before we execute the specified classname (or class corresponding to the keyword) in a thread whose class loader we set up.

I'm inclined to think this is vestigial behavior that quite obscure, has never been defined enough for people to rely on, and has been left in because so few people want to deal with class loader nightmares.


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30382/#review70368
-----------------------------------------------------------


On Jan. 30, 2015, 5:06 p.m., Christopher Tubbs wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30382/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2015, 5:06 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1844 and ACCUMULO-3514
>     https://issues.apache.org/jira/browse/ACCUMULO-1844
>     https://issues.apache.org/jira/browse/ACCUMULO-3514
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
>     ACCUMULO-3514 Use auto-service for start
>     
>     Use @AutoService annotations and Java's ServiceLoader mechanism to discover
>     classes which are executable by Accumulo's "start" jar with a keyword.
>     
>     This replaces manual intervention whenever we add a new option to the
>     bin/accumulo script and also auto-populates the usage for that script.
> 
> 
> Diffs
> -----
> 
>   core/pom.xml 5fc7a6e 
>   core/src/main/java/org/apache/accumulo/core/file/rfile/PrintInfo.java f29efcc 
>   core/src/main/java/org/apache/accumulo/core/util/Classpath.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/util/CreateToken.java 79b241c 
>   core/src/main/java/org/apache/accumulo/core/util/Help.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/util/Jar.java PRE-CREATION 
>   core/src/main/java/org/apache/accumulo/core/util/Version.java ee645ff 
>   minicluster/pom.xml ee6cdc8 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloRunner.java c45abc0 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniClusterExecutable.java PRE-CREATION 
>   pom.xml dda1cfe 
>   proxy/pom.xml 9312d7b 
>   proxy/src/main/java/org/apache/accumulo/proxy/Proxy.java 0a4d12e 
>   server/base/pom.xml c21a168 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java 0a1f411 
>   server/base/src/main/java/org/apache/accumulo/server/util/Admin.java 77d5ea1 
>   server/base/src/main/java/org/apache/accumulo/server/util/Info.java 29fa135 
>   server/base/src/main/java/org/apache/accumulo/server/util/LoginProperties.java be5a7c8 
>   server/base/src/main/java/org/apache/accumulo/server/util/ZooKeeperMain.java 0edcf71 
>   server/gc/pom.xml 9602b95 
>   server/gc/src/main/java/org/apache/accumulo/gc/GCExecutable.java PRE-CREATION 
>   server/master/pom.xml 7e9ab1d 
>   server/master/src/main/java/org/apache/accumulo/master/MasterExecutable.java PRE-CREATION 
>   server/monitor/pom.xml ba61aeb 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/MonitorExecutable.java PRE-CREATION 
>   server/tracer/pom.xml ac9f45f 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/TracerExecutable.java PRE-CREATION 
>   server/tserver/pom.xml cd0f8ef 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TServerExecutable.java PRE-CREATION 
>   shell/pom.xml db3530f 
>   shell/src/main/java/org/apache/accumulo/shell/Shell.java a64ff45 
>   start/src/main/java/org/apache/accumulo/start/Main.java c820883 
>   start/src/main/java/org/apache/accumulo/start/spi/KeywordExecutable.java PRE-CREATION 
>   start/src/test/java/org/apache/accumulo/start/MainTest.java 1ea22fb 
>   test/src/test/java/org/apache/accumulo/start/KeywordStartIT.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30382/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Christopher Tubbs
> 
>


--===============7471909345136893678==--