From 0346b6cdde04233d81a6ec190c3fb6a8eee51d84 Mon Sep 17 00:00:00 2001 From: "qianhao.zhou" Date: Thu, 4 Dec 2014 17:06:12 +0800 Subject: [PATCH 1/2] refactor CliCommandExecutor --- .../java/com/kylinolap/common/util/CliCommandExecutor.java | 13 +++++++------ .../src/main/java/com/kylinolap/dict/lookup/HiveTable.java | 5 +++-- .../java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java | 5 ++--- job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java | 5 ++--- .../com/kylinolap/metadata/tool/HiveSourceTableLoader.java | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java b/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java index 3f4ab7b..31f8d7e 100644 --- a/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java +++ b/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java @@ -73,17 +73,18 @@ private void copyRemote(String localFile, String destDir) throws IOException { } } - public String execute(String command) throws IOException { + public Pair execute(String command) throws IOException { Pair r; - if (remoteHost == null) + if (remoteHost == null) { r = runNativeCommand(command); - else + } else { r = runRemoteCommand(command); + } - if (r.getFirst() != 0) + if (r.getFirst() != 0) { throw new IOException("OS command error exit with " + r.getFirst() + " -- " + command + "\n" + r.getSecond()); - - return r.getSecond(); + } + return r; } private Pair runRemoteCommand(String command) throws IOException { diff --git a/dictionary/src/main/java/com/kylinolap/dict/lookup/HiveTable.java b/dictionary/src/main/java/com/kylinolap/dict/lookup/HiveTable.java index 25d2a87..986a1d6 100644 --- a/dictionary/src/main/java/com/kylinolap/dict/lookup/HiveTable.java +++ b/dictionary/src/main/java/com/kylinolap/dict/lookup/HiveTable.java @@ -91,12 +91,13 @@ private String computeHDFSLocation(boolean needFilePath) throws IOException { String cmd = "hive -e \"describe extended " + hiveTable + ";\""; CliCommandExecutor exec = KylinConfig.getInstanceFromEnv().getCliCommandExecutor(); - String output = exec.execute(cmd); + String output = exec.execute(cmd).getSecond(); Pattern ptn = Pattern.compile("location:(.*?),"); Matcher m = ptn.matcher(output); - if (m.find() == false) + if (m.find() == false) { throw new IOException("Failed to find HDFS location for hive table " + hiveTable + " from output -- " + output); + } String hdfsDir = m.group(1); diff --git a/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java b/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java index c3f9236..a27d94b 100644 --- a/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java +++ b/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java @@ -16,6 +16,7 @@ package com.kylinolap.job.cmd; +import java.io.IOException; import java.util.Map; import org.apache.hadoop.mapreduce.Counters; @@ -124,9 +125,6 @@ private void init() { status = JobStepStatusEnum.NEW; } - /** - * @param jobStatus - */ private void updateHadoopJobInfo() { try { Map jobInfo = job.getInfo(); @@ -185,4 +183,5 @@ private void updateJobCounter() { output.append(e.getLocalizedMessage()); } } + } diff --git a/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java b/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java index 811c06f..aab57ee 100644 --- a/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java +++ b/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java @@ -90,9 +90,8 @@ public Integer call() throws JobException, IOException { protected int executeCommand(String command) throws JobException, IOException { output.reset(); - cliCommandExecutor.execute(command); - //if cliCommandExecutor doesn't throw IOException, it means command is executed correctly - return 0; + output.setStatus(JobStepStatusEnum.RUNNING); + return cliCommandExecutor.execute(command).getFirst(); } @Override diff --git a/metadata/src/main/java/com/kylinolap/metadata/tool/HiveSourceTableLoader.java b/metadata/src/main/java/com/kylinolap/metadata/tool/HiveSourceTableLoader.java index d135729..adcbee1 100644 --- a/metadata/src/main/java/com/kylinolap/metadata/tool/HiveSourceTableLoader.java +++ b/metadata/src/main/java/com/kylinolap/metadata/tool/HiveSourceTableLoader.java @@ -100,7 +100,7 @@ cmd.append("\""); CliCommandExecutor cmdExec = config.getCliCommandExecutor(); - String output = cmdExec.execute(cmd.toString()); + String output = cmdExec.execute(cmd.toString()).getSecond(); return extractTableDescFromHiveOutput(database, output, metaTmpDir); } From dd91f7a2805bd2831dd914eb3816a20bed0ae862 Mon Sep 17 00:00:00 2001 From: "qianhao.zhou" Date: Thu, 4 Dec 2014 17:34:39 +0800 Subject: [PATCH 2/2] fix JavaHadoopCmdOutput cannot parse output issue --- .../kylinolap/common/util/CliCommandExecutor.java | 36 +++++++++++++--------- .../java/com/kylinolap/common/util/Logger.java | 25 +++++++++++++++ .../java/com/kylinolap/common/util/SSHClient.java | 20 +++++------- .../java/com/kylinolap/common/util/SSHLogger.java | 25 --------------- .../com/kylinolap/common/util/SSHClientTest.java | 4 +-- .../com/kylinolap/job/cmd/BaseCommandOutput.java | 12 ++++++++ .../java/com/kylinolap/job/cmd/ICommandOutput.java | 3 +- .../com/kylinolap/job/cmd/JavaHadoopCmdOutput.java | 2 +- .../main/java/com/kylinolap/job/cmd/ShellCmd.java | 2 +- .../java/com/kylinolap/job/cmd/ShellCmdOutput.java | 2 +- 10 files changed, 74 insertions(+), 57 deletions(-) create mode 100644 common/src/main/java/com/kylinolap/common/util/Logger.java delete mode 100644 common/src/main/java/com/kylinolap/common/util/SSHLogger.java create mode 100644 job/src/main/java/com/kylinolap/job/cmd/BaseCommandOutput.java diff --git a/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java b/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java index 31f8d7e..7b0b2f2 100644 --- a/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java +++ b/common/src/main/java/com/kylinolap/common/util/CliCommandExecutor.java @@ -16,9 +16,7 @@ package com.kylinolap.common.util; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; +import java.io.*; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; @@ -63,7 +61,7 @@ private void copyNative(String localFile, String destDir) throws IOException { } private void copyRemote(String localFile, String destDir) throws IOException { - SSHClient ssh = new SSHClient(remoteHost, remoteUser, remotePwd, null); + SSHClient ssh = new SSHClient(remoteHost, remoteUser, remotePwd); try { ssh.scpFileToRemote(localFile, destDir); } catch (IOException e) { @@ -74,11 +72,15 @@ private void copyRemote(String localFile, String destDir) throws IOException { } public Pair execute(String command) throws IOException { + return execute(command, null); + } + + public Pair execute(String command, Logger logAppender) throws IOException { Pair r; if (remoteHost == null) { - r = runNativeCommand(command); + r = runNativeCommand(command, logAppender); } else { - r = runRemoteCommand(command); + r = runRemoteCommand(command, logAppender); } if (r.getFirst() != 0) { @@ -87,12 +89,12 @@ private void copyRemote(String localFile, String destDir) throws IOException { return r; } - private Pair runRemoteCommand(String command) throws IOException { - SSHClient ssh = new SSHClient(remoteHost, remoteUser, remotePwd, null); + private Pair runRemoteCommand(String command, Logger logAppender) throws IOException { + SSHClient ssh = new SSHClient(remoteHost, remoteUser, remotePwd); SSHClientOutput sshOutput; try { - sshOutput = ssh.execCommand(command, remoteTimeoutSeconds); + sshOutput = ssh.execCommand(command, remoteTimeoutSeconds, logAppender); int exitCode = sshOutput.getExitCode(); String output = sshOutput.getText(); return new Pair(exitCode, output); @@ -103,7 +105,7 @@ private void copyRemote(String localFile, String destDir) throws IOException { } } - private Pair runNativeCommand(String command) throws IOException { + private Pair runNativeCommand(String command, Logger logAppender) throws IOException { String[] cmd = new String[3]; String osName = System.getProperty("os.name"); if (osName.startsWith("Windows")) { @@ -119,13 +121,19 @@ private void copyRemote(String localFile, String destDir) throws IOException { builder.redirectErrorStream(true); Process proc = builder.start(); - ByteArrayOutputStream buf = new ByteArrayOutputStream(); - IOUtils.copy(proc.getInputStream(), buf); - String output = buf.toString("UTF-8"); + BufferedReader reader = new BufferedReader(new InputStreamReader(proc.getInputStream())); + String line; + StringBuilder result = new StringBuilder(); + while ((line = reader.readLine()) != null) { + result.append("line").append('\n'); + if (logAppender != null) { + logAppender.log(line); + } + } try { int exitCode = proc.waitFor(); - return new Pair(exitCode, output); + return new Pair(exitCode, result.toString()); } catch (InterruptedException e) { throw new IOException(e); } diff --git a/common/src/main/java/com/kylinolap/common/util/Logger.java b/common/src/main/java/com/kylinolap/common/util/Logger.java new file mode 100644 index 0000000..29033cc --- /dev/null +++ b/common/src/main/java/com/kylinolap/common/util/Logger.java @@ -0,0 +1,25 @@ +/* + * Copyright 2013-2014 eBay Software Foundation + * + * Licensed 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 com.kylinolap.common.util; + +/** + * @author ysong1 + * + */ +public interface Logger { + public void log(String message); +} diff --git a/common/src/main/java/com/kylinolap/common/util/SSHClient.java b/common/src/main/java/com/kylinolap/common/util/SSHClient.java index cea09a7..144adff 100644 --- a/common/src/main/java/com/kylinolap/common/util/SSHClient.java +++ b/common/src/main/java/com/kylinolap/common/util/SSHClient.java @@ -25,7 +25,6 @@ import java.io.InputStream; import java.io.OutputStream; -import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.jcraft.jsch.Channel; @@ -35,16 +34,14 @@ import com.jcraft.jsch.Session; public class SSHClient { - protected static final Logger logger = LoggerFactory.getLogger(SSHClient.class); + protected static final org.slf4j.Logger logger = LoggerFactory.getLogger(SSHClient.class); private String hostname; private String username; private String password; private String identityPath; - private SSHLogger sshLogger; - - public SSHClient(String hostname, String username, String password, SSHLogger sshLogger) { + public SSHClient(String hostname, String username, String password) { this.hostname = hostname; this.username = username; if (new File(password).exists()) { @@ -54,7 +51,6 @@ public SSHClient(String hostname, String username, String password, SSHLogger ss this.password = password; this.identityPath = null; } - this.sshLogger = sshLogger; } public void scpFileToRemote(String localFile, String remoteTargetDirectory) throws Exception { @@ -147,10 +143,10 @@ public void scpFileToRemote(String localFile, String remoteTargetDirectory) thro } public SSHClientOutput execCommand(String command) throws Exception { - return execCommand(command, 7200); + return execCommand(command, 7200, null); } - public SSHClientOutput execCommand(String command, int timeoutSeconds) throws Exception { + public SSHClientOutput execCommand(String command, int timeoutSeconds, Logger logAppender) throws Exception { try { System.out.println("[" + username + "@" + hostname + "] Execute command: " + command); @@ -185,8 +181,8 @@ public SSHClientOutput execCommand(String command, int timeoutSeconds) throws Ex String line = new String(tmp, 0, i); text.append(line); - if (this.sshLogger != null) { - this.sshLogger.log(line); + if (logAppender != null) { + logAppender.log(line); } } while (err.available() > 0) { @@ -196,8 +192,8 @@ public SSHClientOutput execCommand(String command, int timeoutSeconds) throws Ex String line = new String(tmp, 0, i); text.append(line); - if (this.sshLogger != null) { - this.sshLogger.log(line); + if (logAppender != null) { + logAppender.log(line); } } if (channel.isClosed()) { diff --git a/common/src/main/java/com/kylinolap/common/util/SSHLogger.java b/common/src/main/java/com/kylinolap/common/util/SSHLogger.java deleted file mode 100644 index ebf025c..0000000 --- a/common/src/main/java/com/kylinolap/common/util/SSHLogger.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright 2013-2014 eBay Software Foundation - * - * Licensed 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 com.kylinolap.common.util; - -/** - * @author ysong1 - * - */ -public interface SSHLogger { - public void log(String message); -} diff --git a/common/src/test/java/com/kylinolap/common/util/SSHClientTest.java b/common/src/test/java/com/kylinolap/common/util/SSHClientTest.java index 4dd465b..f4ef3df 100644 --- a/common/src/test/java/com/kylinolap/common/util/SSHClientTest.java +++ b/common/src/test/java/com/kylinolap/common/util/SSHClientTest.java @@ -65,7 +65,7 @@ public void testCmd() throws Exception { if (isRemote == false) return; - SSHClient ssh = new SSHClient(this.hostname, this.username, this.password, null); + SSHClient ssh = new SSHClient(this.hostname, this.username, this.password); SSHClientOutput output = ssh.execCommand("echo hello"); assertEquals(0, output.getExitCode()); assertEquals("hello\n", output.getText()); @@ -76,7 +76,7 @@ public void testScp() throws Exception { if (isRemote == false) return; - SSHClient ssh = new SSHClient(this.hostname, this.username, this.password, null); + SSHClient ssh = new SSHClient(this.hostname, this.username, this.password); File tmpFile = FileUtil.createLocalTempFile(new File("/tmp/test_scp"), "temp_", false); ssh.scpFileToRemote(tmpFile.getAbsolutePath(), "/tmp"); } diff --git a/job/src/main/java/com/kylinolap/job/cmd/BaseCommandOutput.java b/job/src/main/java/com/kylinolap/job/cmd/BaseCommandOutput.java new file mode 100644 index 0000000..d68b9d0 --- /dev/null +++ b/job/src/main/java/com/kylinolap/job/cmd/BaseCommandOutput.java @@ -0,0 +1,12 @@ +package com.kylinolap.job.cmd; + +/** + * Created by qianzhou on 12/4/14. + */ +public abstract class BaseCommandOutput implements ICommandOutput { + + @Override + public void log(String message) { + this.appendOutput(message); + } +} diff --git a/job/src/main/java/com/kylinolap/job/cmd/ICommandOutput.java b/job/src/main/java/com/kylinolap/job/cmd/ICommandOutput.java index ca8cc08..65499e1 100644 --- a/job/src/main/java/com/kylinolap/job/cmd/ICommandOutput.java +++ b/job/src/main/java/com/kylinolap/job/cmd/ICommandOutput.java @@ -16,13 +16,14 @@ package com.kylinolap.job.cmd; +import com.kylinolap.common.util.Logger; import com.kylinolap.job.constant.JobStepStatusEnum; /** * @author xjiang * */ -public interface ICommandOutput { +public interface ICommandOutput extends Logger { public void setStatus(JobStepStatusEnum status); diff --git a/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java b/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java index a27d94b..5c296c7 100644 --- a/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java +++ b/job/src/main/java/com/kylinolap/job/cmd/JavaHadoopCmdOutput.java @@ -37,7 +37,7 @@ * @author xduo * */ -public class JavaHadoopCmdOutput implements ICommandOutput { +public class JavaHadoopCmdOutput extends BaseCommandOutput implements ICommandOutput { protected static final Logger log = LoggerFactory.getLogger(JavaHadoopCmdOutput.class); diff --git a/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java b/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java index aab57ee..32c1b21 100644 --- a/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java +++ b/job/src/main/java/com/kylinolap/job/cmd/ShellCmd.java @@ -91,7 +91,7 @@ public Integer call() throws JobException, IOException { protected int executeCommand(String command) throws JobException, IOException { output.reset(); output.setStatus(JobStepStatusEnum.RUNNING); - return cliCommandExecutor.execute(command).getFirst(); + return cliCommandExecutor.execute(command, output).getFirst(); } @Override diff --git a/job/src/main/java/com/kylinolap/job/cmd/ShellCmdOutput.java b/job/src/main/java/com/kylinolap/job/cmd/ShellCmdOutput.java index 7197283..afae59f 100644 --- a/job/src/main/java/com/kylinolap/job/cmd/ShellCmdOutput.java +++ b/job/src/main/java/com/kylinolap/job/cmd/ShellCmdOutput.java @@ -25,7 +25,7 @@ * @author xjiang * */ -public class ShellCmdOutput implements ICommandOutput { +public class ShellCmdOutput extends BaseCommandOutput implements ICommandOutput { protected static final Logger log = LoggerFactory.getLogger(ShellCmdOutput.class);