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


##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java:
##########
@@ -48,13 +49,11 @@
 public abstract class HadoopTask extends AbstractBatchIndexTask
 {
   private static final Logger log = new Logger(HadoopTask.class);
-  private static final ExtensionsConfig EXTENSIONS_CONFIG;
-
-  static final Injector INJECTOR = GuiceInjectors.makeStartupInjector();
 
-  static {
-    EXTENSIONS_CONFIG = INJECTOR.getInstance(ExtensionsConfig.class);
-  }
+  // Note: static variables mean that this task cannot run in a shared JVM,

Review Comment:
   The issue here is more basic: static variables. If task A and task B both 
run in the Indexer, then they share a single copy of the static variable 
(unless the Indexer loads them in isolated class loaders.) By contrast, the 
`ExtensionsLoader` was converted from static variables to a Guice-injected 
singleton to avoid this issue. For `ExtensionsLoader` the only time two 
instances will appear in the same JVM is in tests, which used to have special 
code to undo the static variables between tests.
   
   The `HadoopTask` approach seems more like a bug than a feature. A good 
enhancement would be to modify the `HadoopTask` to use Guice (and the 
`ExtensionsLoader`, perhaps modified for the Hadoop dependencies) to avoid the 
static variable issue. 



-- 
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