From fce795101461cbce37334b0799b2238825f5a5aa Mon Sep 17 00:00:00 2001 From: wenxinhe Date: Mon, 10 Jul 2017 11:37:48 +0800 Subject: [PATCH] HADOOP-14638. Replace commons-logging APIs with slf4j in StreamPumper. This closes #247 Signed-off-by: Akira Ajisaka --- .../apache/hadoop/ha/PowerShellFencer.java | 7 ++- .../apache/hadoop/ha/ShellCommandFencer.java | 7 +-- .../apache/hadoop/ha/SshFenceByTcpPort.java | 7 ++- .../org/apache/hadoop/ha/StreamPumper.java | 8 +-- .../hadoop/ha/TestShellCommandFencer.java | 55 +++++++++++++++++-- 5 files changed, 65 insertions(+), 19 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/PowerShellFencer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/PowerShellFencer.java index 761b40a53a6..6de618c8fcc 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/PowerShellFencer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/PowerShellFencer.java @@ -25,10 +25,10 @@ import java.io.OutputStreamWriter; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configured; import org.apache.hadoop.util.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Fencer method that uses PowerShell to remotely connect to a machine and kill @@ -41,7 +41,8 @@ import org.apache.hadoop.util.StringUtils; */ public class PowerShellFencer extends Configured implements FenceMethod { - private static final Log LOG = LogFactory.getLog(PowerShellFencer.class); + private static final Logger LOG = LoggerFactory.getLogger(PowerShellFencer + .class); @Override diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java index 15edee927de..7e4a88f729f 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/ShellCommandFencer.java @@ -21,12 +21,12 @@ import java.io.IOException; import java.lang.reflect.Field; import java.util.Map; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configured; import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.util.Shell; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Fencing method that runs a shell command. It should be specified @@ -61,8 +61,7 @@ public class ShellCommandFencer private static final String TARGET_PREFIX = "target_"; @VisibleForTesting - static Log LOG = LogFactory.getLog( - ShellCommandFencer.class); + static Logger LOG = LoggerFactory.getLogger(ShellCommandFencer.class); @Override public void checkArgs(String args) throws BadFencingConfigurationException { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/SshFenceByTcpPort.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/SshFenceByTcpPort.java index 58155647d92..64cd5a894c4 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/SshFenceByTcpPort.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/SshFenceByTcpPort.java @@ -32,6 +32,8 @@ import com.jcraft.jsch.ChannelExec; import com.jcraft.jsch.JSch; import com.jcraft.jsch.JSchException; import com.jcraft.jsch.Session; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This fencing implementation sshes to the target node and uses @@ -58,9 +60,8 @@ import com.jcraft.jsch.Session; public class SshFenceByTcpPort extends Configured implements FenceMethod { - static final Log LOG = LogFactory.getLog( - SshFenceByTcpPort.class); - + static final Logger LOG = LoggerFactory.getLogger(SshFenceByTcpPort.class); + static final String CONF_CONNECT_TIMEOUT_KEY = "dfs.ha.fencing.ssh.connect-timeout"; private static final int CONF_CONNECT_TIMEOUT_DEFAULT = diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/StreamPumper.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/StreamPumper.java index 8018f43def3..12a24fd079e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/StreamPumper.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/StreamPumper.java @@ -17,14 +17,14 @@ */ package org.apache.hadoop.ha; +import org.slf4j.Logger; + import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import org.apache.commons.logging.Log; - /** * Class responsible for pumping the streams of the subprocess * out to log4j. stderr is pumped to WARN level and stdout is @@ -35,7 +35,7 @@ class StreamPumper { STDOUT, STDERR; } - private final Log log; + private final Logger log; final Thread thread; final String logPrefix; @@ -43,7 +43,7 @@ class StreamPumper { private final InputStream stream; private boolean started = false; - StreamPumper(final Log log, final String logPrefix, + StreamPumper(final Logger log, final String logPrefix, final InputStream stream, final StreamType type) { this.log = log; this.logPrefix = logPrefix; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java index 6750b3b424e..3a2cf052a60 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestShellCommandFencer.java @@ -19,30 +19,43 @@ package org.apache.hadoop.ha; import static org.junit.Assert.*; +import java.lang.reflect.Method; import java.net.InetSocketAddress; +import java.util.List; +import com.google.common.collect.Lists; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.ha.HAServiceProtocol.HAServiceState; import org.apache.hadoop.util.Shell; import org.apache.hadoop.util.StringUtils; +import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import org.slf4j.Logger; -import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.mock; public class TestShellCommandFencer { private ShellCommandFencer fencer = createFencer(); private static final HAServiceTarget TEST_TARGET = new DummyHAService(HAServiceState.ACTIVE, new InetSocketAddress("dummyhost", 1234)); - + private static final Logger LOG = ShellCommandFencer.LOG; + @BeforeClass - public static void setupLogSpy() { - ShellCommandFencer.LOG = spy(ShellCommandFencer.LOG); + public static void setupLogMock() { + ShellCommandFencer.LOG = mock(Logger.class, new LogAnswer()); } - + + @AfterClass + public static void tearDownLogMock() throws Exception { + ShellCommandFencer.LOG = LOG; + } + @Before public void resetLogSpy() { Mockito.reset(ShellCommandFencer.LOG); @@ -173,4 +186,36 @@ public class TestShellCommandFencer { assertEquals("a...gh", ShellCommandFencer.abbreviate("abcdefgh", 6)); assertEquals("ab...gh", ShellCommandFencer.abbreviate("abcdefgh", 7)); } + + /** + * An answer simply delegate some basic log methods to real LOG. + */ + private static class LogAnswer implements Answer { + + private static final List DELEGATE_METHODS = Lists.asList("error", + new String[]{"warn", "info", "debug", "trace"}); + + @Override + public Object answer(InvocationOnMock invocation) { + + String methodName = invocation.getMethod().getName(); + + if (!DELEGATE_METHODS.contains(methodName)) { + return null; + } + + try { + String msg = invocation.getArguments()[0].toString(); + Method delegateMethod = LOG.getClass().getMethod(methodName, + msg.getClass()); + delegateMethod.invoke(LOG, msg); + } catch (Throwable e) { + throw new IllegalStateException( + "Unsupported delegate method: " + methodName); + } + + return null; + } + } + }