Revert "Add native plan checker to sidecar plugin"

presto-main/src/main/java/com/facebook/presto/connector/ConnectorAwareNodeManager.java

@@ -48,7 +48,6 @@ public Set<Node> getAllNodes()
         return ImmutableSet.<Node>builder()
                 .addAll(getWorkerNodes())
                 .addAll(nodeManager.getCoordinators())
-                .addAll(nodeManager.getCoordinatorSidecars())
                 .build();
     }
 

presto-main/src/main/java/com/facebook/presto/server/PluginManager.java

@@ -354,6 +354,11 @@ public void installPlugin(Plugin plugin)
             log.info("Registering node status notification provider %s", nodeStatusNotificationProviderFactory.getName());
             nodeStatusNotificationManager.addNodeStatusNotificationProviderFactory(nodeStatusNotificationProviderFactory);
         }
+
+        for (PlanCheckerProviderFactory planCheckerProviderFactory : plugin.getPlanCheckerProviderFactories()) {
+            log.info("Registering plan checker provider factory %s", planCheckerProviderFactory.getName());
+            planCheckerProviderManager.addPlanCheckerProviderFactory(planCheckerProviderFactory);
+        }
     }
 
     public void installCoordinatorPlugin(CoordinatorPlugin plugin)
@@ -367,11 +372,6 @@ public void installCoordinatorPlugin(CoordinatorPlugin plugin)
             log.info("Registering system session property provider factory %s", providerFactory.getName());
             metadata.getSessionPropertyManager().addSessionPropertyProviderFactory(providerFactory);
         }
-
-        for (PlanCheckerProviderFactory planCheckerProviderFactory : plugin.getPlanCheckerProviderFactories()) {
-            log.info("Registering plan checker provider factory %s", planCheckerProviderFactory.getName());
-            planCheckerProviderManager.addPlanCheckerProviderFactory(planCheckerProviderFactory);
-        }
     }
 
     private URLClassLoader buildClassLoader(String plugin)

presto-main/src/main/java/com/facebook/presto/server/PrestoServer.java

@@ -26,7 +26,6 @@
 import com.facebook.airlift.json.smile.SmileModule;
 import com.facebook.airlift.log.LogJmxModule;
 import com.facebook.airlift.log.Logger;
-import com.facebook.airlift.node.NodeInfo;
 import com.facebook.airlift.node.NodeModule;
 import com.facebook.airlift.tracetoken.TraceTokenModule;
 import com.facebook.drift.server.DriftServer;
@@ -40,12 +39,9 @@
 import com.facebook.presto.execution.warnings.WarningCollectorModule;
 import com.facebook.presto.metadata.Catalog;
 import com.facebook.presto.metadata.CatalogManager;
-import com.facebook.presto.metadata.DiscoveryNodeManager;
-import com.facebook.presto.metadata.InternalNodeManager;
 import com.facebook.presto.metadata.SessionPropertyManager;
 import com.facebook.presto.metadata.StaticCatalogStore;
 import com.facebook.presto.metadata.StaticFunctionNamespaceStore;
-import com.facebook.presto.nodeManager.PluginNodeManager;
 import com.facebook.presto.security.AccessControlManager;
 import com.facebook.presto.security.AccessControlModule;
 import com.facebook.presto.server.security.PasswordAuthenticatorManager;
@@ -183,13 +179,8 @@ public void run()
             injector.getInstance(TracerProviderManager.class).loadTracerProvider();
             injector.getInstance(NodeStatusNotificationManager.class).loadNodeStatusNotificationProvider();
             injector.getInstance(GracefulShutdownHandler.class).loadNodeStatusNotification();
+            injector.getInstance(PlanCheckerProviderManager.class).loadPlanCheckerProviders();
             injector.getInstance(SessionPropertyManager.class).loadSessionPropertyProviders();
-            PlanCheckerProviderManager planCheckerProviderManager = injector.getInstance(PlanCheckerProviderManager.class);
-            InternalNodeManager nodeManager = injector.getInstance(DiscoveryNodeManager.class);
-            NodeInfo nodeInfo = injector.getInstance(NodeInfo.class);
-            PluginNodeManager pluginNodeManager = new PluginNodeManager(nodeManager, nodeInfo.getEnvironment());
-            planCheckerProviderManager.loadPlanCheckerProviders(pluginNodeManager);
-
             startAssociatedProcesses(injector);
 
             injector.getInstance(Announcer.class).start();

presto-main/src/main/java/com/facebook/presto/sql/planner/sanity/PlanChecker.java

@@ -107,15 +107,7 @@ public void validatePlanFragment(PlanFragment planFragment, Session session, Met
         checkers.get(Stage.FRAGMENT).forEach(checker -> checker.validateFragment(planFragment, session, metadata, warningCollector));
         for (PlanCheckerProvider provider : planCheckerProviderManager.getPlanCheckerProviders()) {
             for (com.facebook.presto.spi.plan.PlanChecker checker : provider.getFragmentPlanCheckers()) {
-                checker.validateFragment(new SimplePlanFragment(
-                        planFragment.getId(),
-                        planFragment.getRoot(),
-                        planFragment.getVariables(),
-                        planFragment.getPartitioning(),
-                        planFragment.getTableScanSchedulingOrder(),
-                        planFragment.getPartitioningScheme(),
-                        planFragment.getStageExecutionDescriptor(),
-                        planFragment.isOutputTableWriterFragment()), warningCollector);
+                checker.validateFragment(toSimplePlanFragment(planFragment), warningCollector);
             }
         }
     }
@@ -134,4 +126,17 @@ private enum Stage
     {
         INTERMEDIATE, FINAL, FRAGMENT
     }
+
+    private static SimplePlanFragment toSimplePlanFragment(PlanFragment planFragment)
+    {
+        return new SimplePlanFragment(
+                planFragment.getId(),
+                planFragment.getRoot(),
+                planFragment.getVariables(),
+                planFragment.getPartitioning(),
+                planFragment.getTableScanSchedulingOrder(),
+                planFragment.getPartitioningScheme(),
+                planFragment.getStageExecutionDescriptor(),
+                planFragment.isOutputTableWriterFragment());
+    }
 }

presto-main/src/main/java/com/facebook/presto/sql/planner/sanity/PlanCheckerProviderManager.java

@@ -14,7 +14,6 @@
 package com.facebook.presto.sql.planner.sanity;
 
 import com.facebook.airlift.log.Logger;
-import com.facebook.presto.spi.NodeManager;
 import com.facebook.presto.spi.plan.PlanCheckerProvider;
 import com.facebook.presto.spi.plan.PlanCheckerProviderContext;
 import com.facebook.presto.spi.plan.PlanCheckerProviderFactory;
@@ -41,15 +40,15 @@ public class PlanCheckerProviderManager
     private static final Logger log = Logger.get(PlanCheckerProviderManager.class);
     private static final String PLAN_CHECKER_PROVIDER_NAME = "plan-checker-provider.name";
 
-    private final SimplePlanFragmentSerde simplePlanFragmentSerde;
+    private final PlanCheckerProviderContext planCheckerProviderContext;
     private final Map<String, PlanCheckerProviderFactory> providerFactories = new ConcurrentHashMap<>();
     private final CopyOnWriteArrayList<PlanCheckerProvider> providers = new CopyOnWriteArrayList<>();
     private final File configDirectory;
 
     @Inject
     public PlanCheckerProviderManager(SimplePlanFragmentSerde simplePlanFragmentSerde, PlanCheckerProviderManagerConfig config)
     {
-        this.simplePlanFragmentSerde = requireNonNull(simplePlanFragmentSerde, "planNodeSerde is null");
+        this.planCheckerProviderContext = new PlanCheckerProviderContext(requireNonNull(simplePlanFragmentSerde, "planNodeSerde is null"));
         requireNonNull(config, "config is null");
         this.configDirectory = requireNonNull(config.getPlanCheckerConfigurationDir(), "configDirectory is null");
     }
@@ -62,11 +61,9 @@ public void addPlanCheckerProviderFactory(PlanCheckerProviderFactory planChecker
         }
     }
 
-    public void loadPlanCheckerProviders(NodeManager nodeManager)
+    public void loadPlanCheckerProviders()
             throws IOException
     {
-        PlanCheckerProviderContext planCheckerProviderContext = new PlanCheckerProviderContext(simplePlanFragmentSerde, nodeManager);
-
         for (File file : listFiles(configDirectory)) {
             if (file.isFile() && file.getName().endsWith(".properties")) {
                 // unlike function namespaces and connectors, we don't have a concept of catalog
@@ -78,7 +75,7 @@ public void loadPlanCheckerProviders(NodeManager nodeManager)
                         file.getAbsoluteFile(),
                         PLAN_CHECKER_PROVIDER_NAME);
                 String planCheckerProviderName = properties.remove(PLAN_CHECKER_PROVIDER_NAME);
-                log.info("-- Loading plan checker provider [%s] --", planCheckerProviderName);
+                log.info("-- Loading plan checker provider %s--", planCheckerProviderName);
                 PlanCheckerProviderFactory providerFactory = providerFactories.get(planCheckerProviderName);
                 checkState(providerFactory != null,
                         "No planCheckerProviderFactory found for '%s'. Available factories were %s", planCheckerProviderName, providerFactories.keySet());

presto-main/src/test/java/com/facebook/presto/sql/planner/sanity/TestPlanCheckerProviderManager.java

@@ -20,7 +20,6 @@
 import com.facebook.presto.spi.plan.PlanCheckerProviderFactory;
 import com.facebook.presto.spi.plan.SimplePlanFragment;
 import com.facebook.presto.sql.planner.plan.JsonCodecSimplePlanFragmentSerde;
-import com.facebook.presto.testing.TestingNodeManager;
 import com.google.common.collect.ImmutableList;
 import org.testng.annotations.Test;
 
@@ -41,7 +40,7 @@ public void testLoadPlanCheckerProviders()
                 .setPlanCheckerConfigurationDir(new File("src/test/resources/plan-checkers"));
         PlanCheckerProviderManager planCheckerProviderManager = new PlanCheckerProviderManager(new JsonCodecSimplePlanFragmentSerde(JsonCodec.jsonCodec(SimplePlanFragment.class)), planCheckerProviderManagerConfig);
         planCheckerProviderManager.addPlanCheckerProviderFactory(new TestingPlanCheckerProviderFactory());
-        planCheckerProviderManager.loadPlanCheckerProviders(new TestingNodeManager());
+        planCheckerProviderManager.loadPlanCheckerProviders();
         assertEquals(planCheckerProviderManager.getPlanCheckerProviders(), ImmutableList.of(TESTING_PLAN_CHECKER_PROVIDER));
     }
 
@@ -52,7 +51,7 @@ public void testLoadUnregisteredPlanCheckerProvider()
         PlanCheckerProviderManagerConfig planCheckerProviderManagerConfig = new PlanCheckerProviderManagerConfig()
                 .setPlanCheckerConfigurationDir(new File("src/test/resources/plan-checkers"));
         PlanCheckerProviderManager planCheckerProviderManager = new PlanCheckerProviderManager(new JsonCodecSimplePlanFragmentSerde(JsonCodec.jsonCodec(SimplePlanFragment.class)), planCheckerProviderManagerConfig);
-        planCheckerProviderManager.loadPlanCheckerProviders(new TestingNodeManager());
+        planCheckerProviderManager.loadPlanCheckerProviders();
     }
 
     public static class TestingPlanCheckerProviderFactory

presto-native-sidecar-plugin/pom.xml

@@ -52,11 +52,6 @@
             <artifactId>log-manager</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>com.squareup.okhttp3</groupId>
-            <artifactId>okhttp</artifactId>
-        </dependency>
-
         <!-- Presto SPI -->
         <dependency>
             <groupId>com.facebook.presto</groupId>
@@ -82,16 +77,6 @@
             <scope>provided</scope>
         </dependency>
 
-        <dependency>
-            <groupId>com.fasterxml.jackson.core</groupId>
-            <artifactId>jackson-databind</artifactId>
-        </dependency>
-
-        <dependency>
-            <groupId>com.fasterxml.jackson.core</groupId>
-            <artifactId>jackson-core</artifactId>
-        </dependency>
-
         <dependency>
             <groupId>com.facebook.drift</groupId>
             <artifactId>drift-api</artifactId>
@@ -110,12 +95,6 @@
             <scope>provided</scope>
         </dependency>
 
-        <dependency>
-            <groupId>com.google.code.findbugs</groupId>
-            <artifactId>jsr305</artifactId>
-            <optional>true</optional>
-        </dependency>
-
         <!-- for testing -->
         <dependency>
             <groupId>com.facebook.presto</groupId>
@@ -203,12 +182,6 @@
             <artifactId>annotations</artifactId>
             <scope>test</scope>
         </dependency>
-
-        <dependency>
-            <groupId>com.squareup.okhttp3</groupId>
-            <artifactId>mockwebserver</artifactId>
-            <scope>test</scope>
-        </dependency>
     </dependencies>
 
     <build>

presto-native-sidecar-plugin/src/main/java/com/facebook/presto/sidecar/NativeSidecarPlugin.java

@@ -13,10 +13,8 @@
  */
 package com.facebook.presto.sidecar;
 
-import com.facebook.presto.sidecar.nativechecker.NativePlanCheckerProviderFactory;
 import com.facebook.presto.sidecar.sessionpropertyproviders.NativeSystemSessionPropertyProviderFactory;
 import com.facebook.presto.spi.CoordinatorPlugin;
-import com.facebook.presto.spi.plan.PlanCheckerProviderFactory;
 import com.facebook.presto.spi.session.WorkerSessionPropertyProviderFactory;
 import com.google.common.collect.ImmutableList;
 
@@ -28,19 +26,4 @@ public Iterable<WorkerSessionPropertyProviderFactory> getWorkerSessionPropertyPr
     {
         return ImmutableList.of(new NativeSystemSessionPropertyProviderFactory());
     }
-
-    @Override
-    public Iterable<PlanCheckerProviderFactory> getPlanCheckerProviderFactories()
-    {
-        return ImmutableList.of(new NativePlanCheckerProviderFactory(getClassLoader()));
-    }
-
-    private static ClassLoader getClassLoader()
-    {
-        ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
-        if (classLoader == null) {
-            classLoader = NativeSidecarPlugin.class.getClassLoader();
-        }
-        return classLoader;
-    }
 }