From ac307fe20d7ce3940955958c0bf91d620f805b9a Mon Sep 17 00:00:00 2001
From: Szilard Nemeth <snemeth@apache.org>
Date: Fri, 5 Jun 2020 12:39:52 +0200
Subject: [PATCH] YARN-10284. Add lazy initialization of
 LogAggregationFileControllerFactory in LogServlet. Contributed by Adam Antal

---
 .../hadoop/yarn/server/webapp/LogServlet.java | 16 ++++--
 .../yarn/server/webapp/TestLogServlet.java    | 52 +++++++++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/webapp/TestLogServlet.java

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/LogServlet.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/LogServlet.java
index 991e9842d0..33de8df011 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/LogServlet.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/LogServlet.java
@@ -62,15 +62,23 @@ public class LogServlet extends Configured {
   private static final Joiner JOINER = Joiner.on("");
   private static final String NM_DOWNLOAD_URI_STR = "/ws/v1/node/containers";
 
-  private final LogAggregationFileControllerFactory factory;
+  private LogAggregationFileControllerFactory factoryInstance = null;
   private final AppInfoProvider appInfoProvider;
 
   public LogServlet(Configuration conf, AppInfoProvider appInfoProvider) {
     super(conf);
-    this.factory = new LogAggregationFileControllerFactory(conf);
     this.appInfoProvider = appInfoProvider;
   }
 
+  private LogAggregationFileControllerFactory getOrCreateFactory() {
+    if (factoryInstance != null) {
+      return factoryInstance;
+    } else {
+      factoryInstance = new LogAggregationFileControllerFactory(getConf());
+      return factoryInstance;
+    }
+  }
+
   @VisibleForTesting
   public String getNMWebAddressFromRM(String nodeId)
       throws ClientHandlerException, UniformInterfaceException, JSONException {
@@ -226,7 +234,7 @@ public Response getContainerLogsInfo(HttpServletRequest req,
       String nmId, boolean redirectedFromNode,
       String clusterId, boolean manualRedirection) {
 
-    builder.setFactory(factory);
+    builder.setFactory(getOrCreateFactory());
 
     BasicAppInfo appInfo;
     try {
@@ -361,6 +369,8 @@ public Response getLogFile(HttpServletRequest req, String containerIdStr,
           "Invalid ContainerId: " + containerIdStr);
     }
 
+    LogAggregationFileControllerFactory factory = getOrCreateFactory();
+
     final long length = LogWebServiceUtils.parseLongParam(size);
 
     ApplicationId appId = containerId.getApplicationAttemptId()
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/webapp/TestLogServlet.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/webapp/TestLogServlet.java
new file mode 100644
index 0000000000..130e154dc1
--- /dev/null
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/webapp/TestLogServlet.java
@@ -0,0 +1,52 @@
+/**
+ * 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.hadoop.yarn.server.webapp;
+
+import org.apache.hadoop.yarn.conf.YarnConfiguration;
+import org.apache.hadoop.yarn.logaggregation.filecontroller.LogAggregationFileControllerFactory;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
+
+public class TestLogServlet {
+  /**
+   * Test that {@link LogServlet}'s constructor does not throw exception,
+   * if the log aggregation properties are bad.
+   */
+  @Test
+  public void testLogServletNoException() {
+    YarnConfiguration conf = new YarnConfiguration();
+    conf.set(YarnConfiguration.LOG_AGGREGATION_FILE_FORMATS, "22");
+
+    // first test the factory's constructor throws exception
+    try {
+      LogAggregationFileControllerFactory factory =
+          new LogAggregationFileControllerFactory(conf);
+      fail("LogAggregationFileControllerFactory should have thrown exception");
+    } catch (IllegalArgumentException expected) {
+    }
+
+    // LogServlet should not throw exception
+    AppInfoProvider aip = mock(AppInfoProvider.class);
+    LogServlet ls = new LogServlet(conf, aip);
+    assertThat(ls).isNotNull();
+  }
+}