(Twill-222) fix for default root log level This closes #37 on Github.
Signed-off-by: Terence Yim <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/twill/repo Commit: http://git-wip-us.apache.org/repos/asf/twill/commit/c4cceef5 Tree: http://git-wip-us.apache.org/repos/asf/twill/tree/c4cceef5 Diff: http://git-wip-us.apache.org/repos/asf/twill/diff/c4cceef5 Branch: refs/heads/site Commit: c4cceef5e797b1d15fe36b09f004c579df10725e Parents: a3e4d38 Author: yaojiefeng <[email protected]> Authored: Thu Mar 16 16:57:05 2017 -0700 Committer: Terence Yim <[email protected]> Committed: Sat Mar 18 01:31:59 2017 -0700 ---------------------------------------------------------------------- .../appmaster/ApplicationMasterService.java | 3 --- .../internal/container/TwillContainerMain.java | 7 +++++- .../apache/twill/yarn/YarnTwillPreparer.java | 3 --- .../org/apache/twill/yarn/LogLevelTestRun.java | 26 +++++++++++--------- 4 files changed, 21 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/twill/blob/c4cceef5/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java ---------------------------------------------------------------------- diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java index b4ac288..368c7b8 100644 --- a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java +++ b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java @@ -650,9 +650,6 @@ public final class ApplicationMasterService extends AbstractYarnTwillService imp String runnableName = provisionRequest.getRuntimeSpec().getName(); LOG.info("Starting runnable {} in {}", runnableName, processLauncher.getContainerInfo().getContainer()); - LOG.debug("Log level for Twill runnable {} is {}", runnableName, - twillRuntimeSpec.getLogLevels().get(runnableName).get(Logger.ROOT_LOGGER_NAME)); - int containerCount = expectedContainers.getExpected(runnableName); // Setup container environment variables http://git-wip-us.apache.org/repos/asf/twill/blob/c4cceef5/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerMain.java ---------------------------------------------------------------------- diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerMain.java b/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerMain.java index e37cd44..a5efb41 100644 --- a/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerMain.java +++ b/twill-yarn/src/main/java/org/apache/twill/internal/container/TwillContainerMain.java @@ -18,6 +18,7 @@ package org.apache.twill.internal.container; import com.google.common.base.Charsets; +import com.google.common.collect.ImmutableMap; import com.google.common.io.Files; import com.google.common.reflect.TypeToken; import com.google.common.util.concurrent.AbstractIdleService; @@ -88,7 +89,11 @@ public final class TwillContainerMain extends ServiceMain { Map<String, String> dynamicLogLevels = loadLogLevels().get(runnableName); Map<String, String> logLevels = new HashMap<>(); - logLevels.putAll(defaultLogLevels); + if (defaultLogLevels != null) { + logLevels.putAll(defaultLogLevels); + } else { + defaultLogLevels = ImmutableMap.of(); + } if (dynamicLogLevels != null) { logLevels.putAll(dynamicLogLevels); } http://git-wip-us.apache.org/repos/asf/twill/blob/c4cceef5/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java ---------------------------------------------------------------------- diff --git a/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java b/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java index d9e70fd..de03a7a 100644 --- a/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java +++ b/twill-yarn/src/main/java/org/apache/twill/yarn/YarnTwillPreparer.java @@ -178,9 +178,6 @@ final class YarnTwillPreparer implements TwillPreparer { this.classAcceptor = new ClassAcceptor(); this.locationCache = locationCache; this.twillClassPaths = twillClassPaths; - - // By default, the root logger is having INFO log level - setLogLevel(LogEntry.Level.INFO); } private void confirmRunnableName(String runnableName) { http://git-wip-us.apache.org/repos/asf/twill/blob/c4cceef5/twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelTestRun.java ---------------------------------------------------------------------- diff --git a/twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelTestRun.java b/twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelTestRun.java index 771797d..717a80f 100644 --- a/twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelTestRun.java +++ b/twill-yarn/src/test/java/org/apache/twill/yarn/LogLevelTestRun.java @@ -39,6 +39,7 @@ import java.io.PrintWriter; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import javax.annotation.Nullable; /** * Test class whether enable certain log level for application container works. @@ -111,11 +112,17 @@ public class LogLevelTestRun extends BaseYarnTest { testLogLevel("ALL"); } + @Test + public void testNoSetLogLevel() throws Exception { + testLogLevel("NONE"); + } + private void testLogLevel(String method) throws Exception { YarnTwillRunnerService runner = getTwillRunner(); runner.start(); TwillPreparer preparer = runner.prepare(new LogLevelTestApplication()); + // Set log level to DEBUG if (method.equals("ROOT")) { preparer.setLogLevel(LogEntry.Level.DEBUG); } @@ -125,7 +132,6 @@ public class LogLevelTestRun extends BaseYarnTest { if (method.equals("RUNNABLE")) { preparer.setLogLevels(LogLevelTestRunnable.class.getSimpleName(), defaultLogArguments); } - // Set log level to DEBUG TwillController controller = preparer .addLogHandler(new PrinterLogHandler(new PrintWriter(System.out))) .start(); @@ -140,11 +146,9 @@ public class LogLevelTestRun extends BaseYarnTest { }, Threads.SAME_THREAD_EXECUTOR); Assert.assertTrue(running.await(200, TimeUnit.SECONDS)); - LogEntry.Level logLevel = waitForLogLevel(controller, LogLevelTestRunnable.class.getSimpleName(), 30L, - TimeUnit.SECONDS); - - // Verify we got DEBUG log level. - Assert.assertEquals(LogEntry.Level.DEBUG, logLevel); + // If we do not set the root log level, it should be null from resource report. + Assert.assertTrue(waitForLogLevel(controller, LogLevelTestRunnable.class.getSimpleName(), 30L, + TimeUnit.SECONDS, !method.equals("NONE") ? LogEntry.Level.DEBUG : null)); controller.terminate().get(120, TimeUnit.SECONDS); @@ -154,8 +158,8 @@ public class LogLevelTestRun extends BaseYarnTest { // Need helper method here to wait for getting resource report because {@link TwillController#getResourceReport()} // could return null if the application has not fully started. - private LogEntry.Level waitForLogLevel(TwillController controller, String runnable, long timeout, - TimeUnit timeoutUnit) throws InterruptedException { + private boolean waitForLogLevel(TwillController controller, String runnable, long timeout, + TimeUnit timeoutUnit, @Nullable LogEntry.Level expected) throws InterruptedException { Stopwatch stopwatch = new Stopwatch(); stopwatch.start(); @@ -166,13 +170,13 @@ public class LogLevelTestRun extends BaseYarnTest { } for (TwillRunResources resources : report.getRunnableResources(runnable)) { LogEntry.Level level = resources.getLogLevels().get(Logger.ROOT_LOGGER_NAME); - if (level != null) { - return level; + if (expected == level) { + return true; } } TimeUnit.MILLISECONDS.sleep(100); } while (stopwatch.elapsedTime(timeoutUnit) < timeout); - return null; + return false; } }
