[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...

2018-01-08 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/1079


---


[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...

2018-01-03 Thread parthchandra
Github user parthchandra commented on a diff in the pull request:

https://github.com/apache/drill/pull/1079#discussion_r159508398
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -344,7 +344,18 @@ private boolean 
clientNeedsAuthExceptPlain(DrillProperties props) {
   mechanismName = factory.getSimpleName();
   logger.trace("Will try to authenticate to server using {} mechanism 
with encryption context {}",
   mechanismName, connection.getEncryptionCtxtString());
+
+  // Update the thread context class loader to current class loader
+  // See DRILL-6063 for detailed description
+  final ClassLoader oldThreadCtxtCL = 
Thread.currentThread().getContextClassLoader();
+  final ClassLoader newThreadCtxtCL = this.getClass().getClassLoader();
+  Thread.currentThread().setContextClassLoader(newThreadCtxtCL);
+
--- End diff --

makes sense


---


[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...

2018-01-03 Thread sohami
Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/1079#discussion_r159500241
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -344,7 +344,18 @@ private boolean 
clientNeedsAuthExceptPlain(DrillProperties props) {
   mechanismName = factory.getSimpleName();
   logger.trace("Will try to authenticate to server using {} mechanism 
with encryption context {}",
   mechanismName, connection.getEncryptionCtxtString());
+
+  // Update the thread context class loader to current class loader
+  // See DRILL-6063 for detailed description
+  final ClassLoader oldThreadCtxtCL = 
Thread.currentThread().getContextClassLoader();
+  final ClassLoader newThreadCtxtCL = this.getClass().getClassLoader();
+  Thread.currentThread().setContextClassLoader(newThreadCtxtCL);
+
--- End diff --

This is the only place since this is when `Hadoop Configuration `object is 
created inside `createAndLoginUser`. SASL authentication mechanism factories 
are scanned before this code itself and moreover they are scanned in context of 
current class ClassLoader not in ThreadContext class loader.


---


[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...

2018-01-03 Thread parthchandra
Github user parthchandra commented on a diff in the pull request:

https://github.com/apache/drill/pull/1079#discussion_r159487621
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -344,7 +344,18 @@ private boolean 
clientNeedsAuthExceptPlain(DrillProperties props) {
   mechanismName = factory.getSimpleName();
   logger.trace("Will try to authenticate to server using {} mechanism 
with encryption context {}",
   mechanismName, connection.getEncryptionCtxtString());
+
+  // Update the thread context class loader to current class loader
+  // See DRILL-6063 for detailed description
+  final ClassLoader oldThreadCtxtCL = 
Thread.currentThread().getContextClassLoader();
+  final ClassLoader newThreadCtxtCL = this.getClass().getClassLoader();
+  Thread.currentThread().setContextClassLoader(newThreadCtxtCL);
+
--- End diff --

Is this the only place we need to update? For eaxmple, with SASL the 
authentication mechanism plugin would have to be in the class path when the 
authentication method is dtermined. But you have already restored the thread 
context. (Or does the oldThreadContextCL have the SASL jars in the classpath?)


---


[GitHub] drill pull request #1079: DRILL-6063: Set correct ThreadContext ClassLoader ...

2018-01-02 Thread sohami
GitHub user sohami opened a pull request:

https://github.com/apache/drill/pull/1079

DRILL-6063: Set correct ThreadContext ClassLoader before using Hadoop…

… Configuration class in DrillClient

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sohami/drill DRILL-6063

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/drill/pull/1079.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1079


commit e7a7273df06f0602c6ca154297b2db01a1181c33
Author: Sorabh Hamirwasia 
Date:   2018-01-02T18:35:19Z

DRILL-6063: Set correct ThreadContext ClassLoader before using Hadoop 
Configuration class in DrillClient




---