Add common DefaultServerHolderModule

Author: abhishekrb19

server/src/main/java/org/apache/druid/guice/DefaultServerHolderModule.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.apache.druid.guice;
+
+import com.google.inject.Binder;
+import com.google.inject.Module;
+import org.apache.druid.server.metrics.DefaultLoadSpecHolder;
+import org.apache.druid.server.metrics.LoadSpecHolder;
+import org.apache.druid.server.metrics.NoopTaskHolder;
+import org.apache.druid.server.metrics.TaskHolder;
+
+/**
+ * Binds the following holder configs for all services:
+ * <ul>
+ * <li>{@link TaskHolder} to {@link NoopTaskHolder}</li>
+ * <li>{@link LoadSpecHolder} to {@link DefaultLoadSpecHolder}</li>
+ * </ul>
+ *
+ * <p>For {@code CliPeon}, these bindings are overridden by the peon-specific module.</p>
+ */
+public class DefaultServerHolderModule implements Module
+{
+  @Override
+  public void configure(Binder binder)
+  {
+    binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
+    binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
+  }
+}

server/src/main/java/org/apache/druid/initialization/CoreInjectorBuilder.java

@@ -26,6 +26,7 @@
 import org.apache.druid.guice.AnnouncerModule;
 import org.apache.druid.guice.BuiltInTypesModule;
 import org.apache.druid.guice.CatalogCoreModule;
+import org.apache.druid.guice.DefaultServerHolderModule;
 import org.apache.druid.guice.DruidInjectorBuilder;
 import org.apache.druid.guice.DruidSecondaryModule;
 import org.apache.druid.guice.ExpressionModule;
@@ -142,7 +143,8 @@ public CoreInjectorBuilder forServerWithoutJetty()
         new ExternalStorageAccessSecurityModule(),
         new ServiceClientModule(),
         new StorageConnectorModule(),
-        new CatalogCoreModule()
+        new CatalogCoreModule(),
+        new DefaultServerHolderModule()
     );
     return this;
   }

server/src/main/java/org/apache/druid/server/metrics/MetricsModule.java

@@ -108,7 +108,7 @@ public MonitorScheduler getMonitorScheduler(
   {
     List<Monitor> monitors = new ArrayList<>();
     // HACK: when ServiceStatusMonitor is the first to be loaded, it introduces a circular dependency between
-    // CliPeon.runTask and CliPeon.getTaskHolder/CliPeon.getCliPeonLoadSpecHolder. The reason for this is unclear
+    // CliPeon.runTask and TaskHolder.getDataSource()/TaskHolder.getTaskId(). The reason for this is unclear
     // but by injecting TaskPropertiesHolder early this cycle is avoided.
     injector.getInstance(TaskHolder.class);
     injector.getInstance(LoadSpecHolder.class);

server/src/main/java/org/apache/druid/server/metrics/NoopTaskHolder.java

@@ -32,12 +32,12 @@ public class NoopTaskHolder implements TaskHolder
   @Override
   public String getDataSource()
   {
-    throw new UnsupportedOperationException("getDataSource() should not be called for non-cli peons");
+    throw new UnsupportedOperationException("getDataSource() should not be called for non-CliPeon");
   }
 
   @Override
   public String getTaskId()
   {
-    throw new UnsupportedOperationException("getTaskId() should not be called for non-cli peons");
+    throw new UnsupportedOperationException("getTaskId() should not be called for non-CliPeon");
   }
 }

server/src/test/java/org/apache/druid/server/metrics/MetricsModuleTest.java

@@ -88,8 +88,6 @@ public void configure(Binder binder)
                 Key.get(DruidNode.class, Self.class),
                 new DruidNode("test-inject", null, false, null, null, true, false)
             );
-            binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
-            binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
           }
         })
     );

services/src/main/java/org/apache/druid/cli/CliBroker.java

@@ -82,12 +82,8 @@
 import org.apache.druid.server.http.SegmentListerResource;
 import org.apache.druid.server.http.SelfDiscoveryResource;
 import org.apache.druid.server.initialization.jetty.JettyServerInitializer;
-import org.apache.druid.server.metrics.DefaultLoadSpecHolder;
-import org.apache.druid.server.metrics.LoadSpecHolder;
-import org.apache.druid.server.metrics.NoopTaskHolder;
 import org.apache.druid.server.metrics.QueryCountStatsProvider;
 import org.apache.druid.server.metrics.SubqueryCountStatsProvider;
-import org.apache.druid.server.metrics.TaskHolder;
 import org.apache.druid.server.router.TieredBrokerConfig;
 import org.apache.druid.sql.calcite.schema.MetadataSegmentView;
 import org.apache.druid.sql.guice.SqlModule;
@@ -165,9 +161,6 @@ protected List<? extends Module> getModules()
           JsonConfigProvider.bind(binder, "druid.broker.internal.query.config", InternalQueryConfig.class);
 
           binder.bind(QuerySegmentWalker.class).to(ClientQuerySegmentWalker.class).in(LazySingleton.class);
-          binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
-          binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
-
           binder.bind(JettyServerInitializer.class).to(QueryJettyServerInitializer.class).in(LazySingleton.class);
 
           binder.bind(BrokerQueryResource.class).in(LazySingleton.class);

services/src/main/java/org/apache/druid/cli/CliCoordinator.java

@@ -105,11 +105,7 @@
 import org.apache.druid.server.initialization.jetty.JettyServerInitializer;
 import org.apache.druid.server.lookup.cache.LookupCoordinatorManager;
 import org.apache.druid.server.lookup.cache.LookupCoordinatorManagerConfig;
-import org.apache.druid.server.metrics.DefaultLoadSpecHolder;
-import org.apache.druid.server.metrics.LoadSpecHolder;
-import org.apache.druid.server.metrics.NoopTaskHolder;
 import org.apache.druid.server.metrics.ServiceStatusMonitor;
-import org.apache.druid.server.metrics.TaskHolder;
 import org.apache.druid.server.router.TieredBrokerConfig;
 import org.apache.druid.storage.local.LocalTmpStorageConfig;
 import org.eclipse.jetty.server.Server;
@@ -203,9 +199,6 @@ public void configure(Binder binder)
             JsonConfigProvider.bind(binder, "druid.coordinator.segmentMetadataCache", SegmentMetadataCacheConfig.class);
             binder.bind(DruidCoordinatorConfig.class);
 
-            binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
-            binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
-
             binder.bind(RedirectFilter.class).in(LazySingleton.class);
             binder.bind(CoordinatorDynamicConfigSyncer.class).in(ManageLifecycle.class);
             if (beOverlord) {

services/src/main/java/org/apache/druid/cli/CliHistorical.java

@@ -63,11 +63,7 @@
 import org.apache.druid.server.http.SegmentListerResource;
 import org.apache.druid.server.http.SelfDiscoveryResource;
 import org.apache.druid.server.initialization.jetty.JettyServerInitializer;
-import org.apache.druid.server.metrics.DefaultLoadSpecHolder;
-import org.apache.druid.server.metrics.LoadSpecHolder;
-import org.apache.druid.server.metrics.NoopTaskHolder;
 import org.apache.druid.server.metrics.QueryCountStatsProvider;
-import org.apache.druid.server.metrics.TaskHolder;
 import org.apache.druid.storage.local.LocalTmpStorageConfig;
 import org.apache.druid.timeline.PruneLastCompactionState;
 import org.eclipse.jetty.server.Server;
@@ -125,8 +121,6 @@ protected List<? extends Module> getModules()
           binder.bind(ServerManager.class).in(LazySingleton.class);
           binder.bind(SegmentManager.class).in(LazySingleton.class);
           binder.bind(ZkCoordinator.class).in(ManageLifecycle.class);
-          binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
-          binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
           bindQuerySegmentWalker(binder);
 
           binder.bind(ServerTypeConfig.class).toInstance(new ServerTypeConfig(ServerType.HISTORICAL));

services/src/main/java/org/apache/druid/cli/CliMiddleManager.java

@@ -83,11 +83,7 @@
 import org.apache.druid.server.DruidNode;
 import org.apache.druid.server.http.SelfDiscoveryResource;
 import org.apache.druid.server.initialization.jetty.JettyServerInitializer;
-import org.apache.druid.server.metrics.DefaultLoadSpecHolder;
-import org.apache.druid.server.metrics.LoadSpecHolder;
-import org.apache.druid.server.metrics.NoopTaskHolder;
 import org.apache.druid.server.metrics.ServiceStatusMonitor;
-import org.apache.druid.server.metrics.TaskHolder;
 import org.apache.druid.server.metrics.WorkerTaskCountStatsProvider;
 import org.apache.druid.storage.local.LocalTmpStorageConfig;
 import org.apache.druid.timeline.PruneLastCompactionState;
@@ -154,8 +150,6 @@ public void configure(Binder binder)
             binder.bind(TaskRunner.class).to(ForkingTaskRunner.class);
             binder.bind(ForkingTaskRunner.class).in(ManageLifecycle.class);
             binder.bind(WorkerTaskCountStatsProvider.class).to(ForkingTaskRunner.class);
-            binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
-            binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
 
             binder.bind(ParallelIndexSupervisorTaskClientProvider.class).toProvider(Providers.of(null));
             binder.bind(ShuffleClient.class).toProvider(Providers.of(null));

services/src/main/java/org/apache/druid/cli/CliRouter.java

@@ -46,11 +46,7 @@
 import org.apache.druid.server.http.RouterResource;
 import org.apache.druid.server.http.SelfDiscoveryResource;
 import org.apache.druid.server.initialization.jetty.JettyServerInitializer;
-import org.apache.druid.server.metrics.DefaultLoadSpecHolder;
-import org.apache.druid.server.metrics.LoadSpecHolder;
-import org.apache.druid.server.metrics.NoopTaskHolder;
 import org.apache.druid.server.metrics.QueryCountStatsProvider;
-import org.apache.druid.server.metrics.TaskHolder;
 import org.apache.druid.server.router.AvaticaConnectionBalancer;
 import org.apache.druid.server.router.CoordinatorRuleManager;
 import org.apache.druid.server.router.ManagementProxyConfig;
@@ -112,9 +108,6 @@ protected List<? extends Module> getModules()
           binder.bind(CoordinatorRuleManager.class);
           LifecycleModule.register(binder, CoordinatorRuleManager.class);
 
-          binder.bind(TaskHolder.class).to(NoopTaskHolder.class).in(LazySingleton.class);
-          binder.bind(LoadSpecHolder.class).to(DefaultLoadSpecHolder.class).in(LazySingleton.class);
-
           binder.bind(TieredBrokerHostSelector.class).in(ManageLifecycle.class);
           binder.bind(QueryHostFinder.class).in(LazySingleton.class);
           binder.bind(new TypeLiteral<List<TieredBrokerSelectorStrategy>>() {})