simplify as log4j2.xml is the real switch for log routing

apache · Sep 15, 2022 · 893870f · 893870f
1 parent 86847fb
commit 893870f
Show file tree

Hide file tree

Showing 4 changed files with 4 additions and 25 deletions.
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
@@ -75,8 +75,4 @@ public String getMetricsPrefix() {
         .orElseGet(() -> getProperty(CommonConstants.Minion.DEPRECATED_CONFIG_OF_METRICS_PREFIX_KEY,
             CommonConstants.Minion.CONFIG_OF_METRICS_PREFIX));
   }
-
-  public boolean isSeparateTaskLogs() {
-    return getProperty(CommonConstants.Minion.CONFIG_OF_SEPARATE_TASK_LOGS, false);
-  }
 }
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/executor/TaskExecutorFactoryRegistry.java b/pinot-minion/src/main/java/org/apache/pinot/minion/executor/TaskExecutorFactoryRegistry.java
@@ -41,8 +41,6 @@ public class TaskExecutorFactoryRegistry {
 
   private final Map<String, PinotTaskExecutorFactory> _taskExecutorFactoryRegistry = new HashMap<>();
 
-  private final MinionConf _minionConf;
-
   /**
    * Registers the task executor factories via reflection.
    * NOTE: In order to plugin a class using reflection, the class should include ".plugin.minion.tasks." in its class
@@ -64,16 +62,11 @@ public TaskExecutorFactoryRegistry(MinionTaskZkMetadataManager zkMetadataManager
         }
       }
     }
-    _minionConf = minionConf;
     LOGGER.info("Initialized TaskExecutorFactoryRegistry with {} task executor factories: {} in {}ms",
         _taskExecutorFactoryRegistry.size(), _taskExecutorFactoryRegistry.keySet(),
         System.currentTimeMillis() - startTimeMs);
   }
 
-  public MinionConf getMinionConf() {
-    return _minionConf;
-  }
-
   public static Set<Class<?>> getTaskExecutorFactoryClasses() {
     return PinotReflectionUtils
         .getClassesThroughReflection(TASK_EXECUTOR_FACTORY_PACKAGE_REGEX_PATTERN, TaskExecutorFactory.class);

diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java b/pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
@@ -36,7 +36,6 @@
 import org.apache.pinot.common.metrics.MinionQueryPhase;
 import org.apache.pinot.core.common.MinionConstants;
 import org.apache.pinot.core.minion.PinotTaskConfig;
-import org.apache.pinot.minion.MinionConf;
 import org.apache.pinot.minion.MinionContext;
 import org.apache.pinot.minion.event.EventObserverFactoryRegistry;
 import org.apache.pinot.minion.event.MinionEventObserver;
@@ -62,8 +61,6 @@ public class TaskFactoryRegistry {
 
   public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegistry,
       EventObserverFactoryRegistry eventObserverFactoryRegistry) {
-    MinionConf minionConf = taskExecutorFactoryRegistry.getMinionConf();
-    boolean separateTaskLogs = minionConf.isSeparateTaskLogs();
     for (String taskType : taskExecutorFactoryRegistry.getAllTaskTypes()) {
       PinotTaskExecutorFactory taskExecutorFactory = taskExecutorFactoryRegistry.getTaskExecutorFactory(taskType);
       MinionEventObserverFactory eventObserverFactory = eventObserverFactoryRegistry.getEventObserverFactory(taskType);
@@ -89,11 +86,8 @@ public TaskResult run() {
                   .addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTimeMs - jobInQueueTimeMs,
                       TimeUnit.MILLISECONDS);
               try {
-                if (separateTaskLogs) {
-                  LOGGER.info("Routing logs to separate file for task: {}", _taskConfig.getId());
-                  // Setting MDC so that logger can route task logs to a separate file.
-                  MDC.put("taskId", _taskConfig.getId());
-                }
+                // Set taskId in MDC so that one may config logger to route task logs to separate file.
+                MDC.put("taskId", _taskConfig.getId());
                 _minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, 1L);
                 return runInternal();
               } finally {
@@ -102,11 +96,8 @@ public TaskResult run() {
                 _minionMetrics
                     .addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, executionTimeMs, TimeUnit.MILLISECONDS);
                 LOGGER.info("Task: {} completed in: {}ms", _taskConfig.getId(), executionTimeMs);
-                if (separateTaskLogs) {
-                  // Clearing MDC to stop routing logs to a separate file.
-                  MDC.remove("taskId");
-                  LOGGER.info("All logs routed to separate file for task: {}", _taskConfig.getId());
-                }
+                // Clear taskId from MDC to reset it.
+                MDC.remove("taskId");
               }
             }
 

diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -583,7 +583,6 @@ public static class Minion {
     public static final String MINION_TLS_PREFIX = "pinot.minion.tls";
     public static final String CONFIG_OF_MINION_QUERY_REWRITER_CLASS_NAMES = "pinot.minion.query.rewriter.class.names";
     public static final String CONFIG_OF_LOGGER_ROOT_DIR = "pinot.minion.logger.root.dir";
-    public static final String CONFIG_OF_SEPARATE_TASK_LOGS = "pinot.minion.separate.task.logs";
   }
 
   public static class ControllerJob {