paul-rogers commented on code in PR #12827:
URL: https://github.com/apache/druid/pull/12827#discussion_r931521115


##########
extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/ParquetInputFormat.java:
##########
@@ -51,6 +53,26 @@ public ParquetInputFormat(
     this.conf = conf;
   }
 
+  private void initialize(Configuration conf)
+  {
+    //Initializing seperately since during eager initialization, resolving
+    //namenode hostname throws an error if nodes are ephemeral
+
+    // Ensure that FileSystem class level initialization happens with correct 
CL
+    // See https://github.com/apache/druid/issues/1714
+    ClassLoader currCtxCl = Thread.currentThread().getContextClassLoader();
+    try {
+      
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
+      FileSystem.get(conf);
+    }
+    catch (IOException ex) {
+      throw new RuntimeException(ex);
+    }
+    finally {
+      Thread.currentThread().setContextClassLoader(currCtxCl);
+    }

Review Comment:
   This seems a step in the right direction. Is it enough?
   
   I the NN is ephemeral, isn't there still a race condition between here and 
when we actually resolve a file name using the NN? Do we have to repeat this 
for each NN change? Should it occur just prior to the first file reference? 
Should we then retry in case of a NN failover at that moment?
   
   Also, this way of initializing with the target class loader works, but is 
odd. First, what does the class loader do? By calling `getClass()`, we're 
getting the class loader for this class. But, we already have that since we are 
in this class. Did we mean the class loader for the extensions module?
   
   Can we have an `Initialization` class loaded in that class loader (assuming 
Druid code runs in that class loader), and invoke that so we get the class 
loader set up automagically?



##########
extensions-core/parquet-extensions/src/main/java/org/apache/druid/data/input/parquet/ParquetInputFormat.java:
##########
@@ -51,6 +53,26 @@ public ParquetInputFormat(
     this.conf = conf;
   }
 
+  private void initialize(Configuration conf)
+  {
+    //Initializing seperately since during eager initialization, resolving
+    //namenode hostname throws an error if nodes are ephemeral

Review Comment:
   Nit: space after `//`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to