From c1a48110eed5d666b6fe5c95891020abe1f4cba9 Mon Sep 17 00:00:00 2001 From: Adam Antal Date: Wed, 20 May 2020 14:57:23 +0200 Subject: [PATCH] YARN-10284. Add lazy initialization of LogAggregationFileControllerFactory in LogServlet --- .../hadoop/yarn/server/webapp/LogServlet.java | 20 +++++-- .../yarn/server/webapp/TestLogServlet.java | 52 +++++++++++++++++++ 2 files changed, 69 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 991e9842d0ea88fcba6f246b650003d92a1b0984..4e263038c8ebad1da44b5e2a27feead310d5f04e 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 @@ -49,6 +49,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; /** * Extracts aggregated logs and related information. @@ -62,15 +63,26 @@ private static final Joiner JOINER = Joiner.on(""); private static final String NM_DOWNLOAD_URI_STR = "/ws/v1/node/containers"; - private final LogAggregationFileControllerFactory factory; + private Optional factory; private final AppInfoProvider appInfoProvider; public LogServlet(Configuration conf, AppInfoProvider appInfoProvider) { super(conf); - this.factory = new LogAggregationFileControllerFactory(conf); + this.factory = Optional.empty(); this.appInfoProvider = appInfoProvider; } + private LogAggregationFileControllerFactory getOrCreateFactory() { + if (factory.isPresent()) { + return factory.get(); + } else { + LogAggregationFileControllerFactory f = + new LogAggregationFileControllerFactory(getConf()); + factory = Optional.of(f); + return f; + } + } + @VisibleForTesting public String getNMWebAddressFromRM(String nodeId) throws ClientHandlerException, UniformInterfaceException, JSONException { @@ -226,7 +238,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 +373,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 0000000000000000000000000000000000000000..130e154dc1e07e32e41dfa76f977219c3714c0ee --- /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(); + } +} -- 2.21.0