From c89bdfacc8715fa6d72acd85437ab8cd257c8aad Mon Sep 17 00:00:00 2001
From: Adam Antal <adam.antal@cloudera.com>
Date: Wed, 14 Aug 2019 17:35:16 +0200
Subject: [PATCH] =?UTF-8?q?YARN-9676.=20Add=20DEBUG=20and=20TRACE=20level?=
 =?UTF-8?q?=20messages=20to=20AppLogAggregatorImpl=E2=80=A6=20(#1261)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* YARN-9676. Add DEBUG and TRACE level messages to AppLogAggregatorImpl and connected classes

* Using {} placeholder, and increasing loglevel if log aggregation failed.
---
 .../logaggregation/AppLogAggregatorImpl.java  | 42 +++++++++++--------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java
index ef14d2aba8f..77bec7237e0 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AppLogAggregatorImpl.java
@@ -108,7 +108,6 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
   // These variables are only for testing
   private final AtomicBoolean waiting = new AtomicBoolean(false);
   private int logAggregationTimes = 0;
-  private int cleanupOldLogTimes = 0;
   private long logFileSizeThreshold;
   private boolean renameTemporaryLogFileFailed = false;
 
@@ -196,10 +195,16 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
       this.remoteNodeTmpLogFileForApp = getRemoteNodeTmpLogFileForApp();
     }
     boolean logAggregationInRolling =
-        rollingMonitorInterval <= 0 || this.logAggregationContext == null
-            || this.logAggregationContext.getRolledLogsIncludePattern() == null
-            || this.logAggregationContext.getRolledLogsIncludePattern()
-                .isEmpty() ? false : true;
+        rollingMonitorInterval > 0 && this.logAggregationContext != null
+            && this.logAggregationContext.getRolledLogsIncludePattern() != null
+            && !this.logAggregationContext.getRolledLogsIncludePattern()
+                .isEmpty();
+    if (logAggregationInRolling) {
+      LOG.info("Rolling mode is turned on with include pattern {}",
+          this.logAggregationContext.getRolledLogsIncludePattern());
+    } else {
+      LOG.debug("Rolling mode is turned off");
+    }
     logControllerContext = new LogAggregationFileControllerContext(
             this.remoteNodeLogFileForApp,
             this.remoteNodeTmpLogFileForApp,
@@ -299,11 +304,13 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
     }
 
     if (pendingContainerInThisCycle.isEmpty()) {
+      LOG.debug("No pending container in this cycle");
       sendLogAggregationReport(true, "", appFinished);
       return;
     }
 
     logAggregationTimes++;
+    LOG.debug("Cycle #{} of log aggregator", logAggregationTimes);
     String diagnosticMessage = "";
     boolean logAggregationSucceedInThisCycle = true;
     DeletionTask deletionTask = null;
@@ -331,6 +338,8 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
             appFinished, finishedContainers.contains(container));
         if (uploadedFilePathsInThisCycle.size() > 0) {
           uploadedLogsInThisCycle = true;
+          LOG.trace("Uploaded the following files for {}: {}",
+              container, uploadedFilePathsInThisCycle.toString());
           List<Path> uploadedFilePathsInThisCycleList = new ArrayList<>();
           uploadedFilePathsInThisCycleList.addAll(uploadedFilePathsInThisCycle);
           if (LOG.isDebugEnabled()) {
@@ -386,6 +395,13 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
       if (logAggregationSucceedInThisCycle && deletionTask != null) {
         delService.delete(deletionTask);
       }
+      if (!diagnosticMessage.isEmpty()) {
+        LOG.debug("Sending log aggregation report along with the " +
+            "following diagnostic message:\"{}\"", diagnosticMessage);
+      }
+      if (!logAggregationSucceedInThisCycle) {
+        LOG.warn("Log aggregation did not succeed in this cycle");
+      }
       sendLogAggregationReport(logAggregationSucceedInThisCycle,
           diagnosticMessage, appFinished);
       if (exc != null) {
@@ -522,14 +538,17 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
         lfs.getFileStatus(logPath);
         localAppLogDirs.add(logPath);
       } catch (UnsupportedFileSystemException ue) {
-        LOG.warn("Log dir " + rootLogDir + "is an unsupported file system", ue);
+        LOG.warn("Log dir {} is in an unsupported file system", rootLogDir,
+            ue);
         continue;
       } catch (IOException fe) {
+        LOG.warn("An exception occurred while getting file information", fe);
         continue;
       }
     }
 
     if (localAppLogDirs.size() > 0) {
+      LOG.debug("Cleaning up {} files", localAppLogDirs.size());
       List<Path> localAppLogDirsList = new ArrayList<>();
       localAppLogDirsList.addAll(localAppLogDirs);
       DeletionTask deletionTask = new FileDeletionTask(delService,
@@ -672,17 +691,6 @@ public class AppLogAggregatorImpl implements AppLogAggregator {
     return userUgi;
   }
 
-  @Private
-  @VisibleForTesting
-  public int getLogAggregationTimes() {
-    return this.logAggregationTimes;
-  }
-
-  @VisibleForTesting
-  int getCleanupOldLogTimes() {
-    return this.cleanupOldLogTimes;
-  }
-
   @VisibleForTesting
   public LogAggregationFileController getLogAggregationFileController() {
     return this.logAggregationFileController;