Skip to content

Commit

Permalink
Address review comments regarding exception handling.
Browse files Browse the repository at this point in the history
  • Loading branch information
riversand9 committed Aug 29, 2017
1 parent f377de4 commit 878c3b1
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 25 deletions.
2 changes: 1 addition & 1 deletion core/server/common/src/main/java/alluxio/ProcessUtils.java
Expand Up @@ -59,7 +59,7 @@ public void run() {
try { try {
process.stop(); process.stop();
} catch (Exception e) { } catch (Exception e) {
LOG.error("Failed to shutdown process."); LOG.error("Failed to shutdown process.", e);
System.exit(0); System.exit(0);
} }
} }
Expand Down
Expand Up @@ -42,7 +42,7 @@
public class AlluxioLog4jSocketNode implements Runnable { public class AlluxioLog4jSocketNode implements Runnable {
private static final org.slf4j.Logger LOG = private static final org.slf4j.Logger LOG =
org.slf4j.LoggerFactory.getLogger(AlluxioLog4jSocketNode.class); org.slf4j.LoggerFactory.getLogger(AlluxioLog4jSocketNode.class);
private final AlluxioLogServerProcess mLogServerProcess; private final String mBaseLogsDir;
private final Socket mSocket; private final Socket mSocket;


/** /**
Expand All @@ -51,12 +51,12 @@ public class AlluxioLog4jSocketNode implements Runnable {
* Callers construct AlluxioLog4jSocketNode instances, passing the ownership of the socket * Callers construct AlluxioLog4jSocketNode instances, passing the ownership of the socket
* parameter. From now on, the AlluxioLog4jSocketNode is responsible for closing the socket. * parameter. From now on, the AlluxioLog4jSocketNode is responsible for closing the socket.
* *
* @param process main log server process * @param baseLogsDir base directory for logs
* @param socket client socket from which to read {@link org.apache.log4j.spi.LoggingEvent} * @param socket client socket from which to read {@link org.apache.log4j.spi.LoggingEvent}
*/ */
public AlluxioLog4jSocketNode(AlluxioLogServerProcess process, Socket socket) { public AlluxioLog4jSocketNode(String baseLogsDir, Socket socket) {
mLogServerProcess = Preconditions.checkNotNull(process, mBaseLogsDir = Preconditions.checkNotNull(baseLogsDir,
"The log server process cannot be null."); "Base logs directory cannot be null.");
mSocket = Preconditions.checkNotNull(socket, "Client socket cannot be null"); mSocket = Preconditions.checkNotNull(socket, "Client socket cannot be null");
} }


Expand All @@ -70,10 +70,10 @@ public void run() {
while (!Thread.currentThread().isInterrupted()) { while (!Thread.currentThread().isInterrupted()) {
try { try {
event = (LoggingEvent) objectInputStream.readObject(); event = (LoggingEvent) objectInputStream.readObject();
} catch (ClassNotFoundException e1) { } catch (ClassNotFoundException e) {
throw new RuntimeException("Class of serialized object cannot be found.", e1); throw new RuntimeException("Class of serialized object cannot be found.", e);
} catch (IOException e1) { } catch (IOException e) {
throw new RuntimeException("Cannot read object from stream due to I/O error.", e1); throw new RuntimeException("Cannot read object from stream due to I/O error.", e);
} }
if (hierarchy == null) { if (hierarchy == null) {
hierarchy = configureHierarchy( hierarchy = configureHierarchy(
Expand All @@ -86,7 +86,7 @@ public void run() {
} }
} catch (IOException e) { } catch (IOException e) {
// Something went wrong, cannot recover. // Something went wrong, cannot recover.
throw new RuntimeException("Cannot open ObjectInputStream due to I/O error.", e); throw new RuntimeException(e);
} finally { } finally {
try { try {
mSocket.close(); mSocket.close();
Expand All @@ -102,11 +102,11 @@ public void run() {
* hierarchy. {@link AlluxioLog4jSocketNode} instance can retrieve the logger to log incoming * hierarchy. {@link AlluxioLog4jSocketNode} instance can retrieve the logger to log incoming
* {@link org.apache.log4j.spi.LoggingEvent}s. * {@link org.apache.log4j.spi.LoggingEvent}s.
* *
* @param logAppenderName name of the appender to use for this client * @param processType type of the process sending this {@link LoggingEvent}
* @return a {@link Hierarchy} instance to retrieve logger * @return a {@link Hierarchy} instance to retrieve logger
* @throws IOException if fails to create an {@link FileInputStream} to read log4j.properties * @throws IOException if fails to create an {@link FileInputStream} to read log4j.properties
*/ */
private LoggerRepository configureHierarchy(String logAppenderName) private LoggerRepository configureHierarchy(String processType)
throws IOException { throws IOException {
String inetAddressStr = mSocket.getInetAddress().getHostAddress(); String inetAddressStr = mSocket.getInetAddress().getHostAddress();
Properties properties = new Properties(); Properties properties = new Properties();
Expand All @@ -130,7 +130,7 @@ private LoggerRepository configureHierarchy(String logAppenderName)
Hierarchy clientHierarchy = new Hierarchy(new RootLogger(level)); Hierarchy clientHierarchy = new Hierarchy(new RootLogger(level));
// Startup script should guarantee that mBaseLogsDir already exists. // Startup script should guarantee that mBaseLogsDir already exists.
String logDirectoryPath = String logDirectoryPath =
PathUtils.concatPath(mLogServerProcess.getBaseLogsDir(), logAppenderName.toLowerCase()); PathUtils.concatPath(mBaseLogsDir, processType.toLowerCase());
File logDirectory = new File(logDirectoryPath); File logDirectory = new File(logDirectoryPath);
if (!logDirectory.exists()) { if (!logDirectory.exists()) {
logDirectory.mkdir(); logDirectory.mkdir();
Expand Down
Expand Up @@ -47,9 +47,6 @@ public class AlluxioLogServerProcess implements Process {
public static final String LOGSERVER_CLIENT_LOGGER_APPENDER_NAME = "LOGSERVER_CLIENT_LOGGER"; public static final String LOGSERVER_CLIENT_LOGGER_APPENDER_NAME = "LOGSERVER_CLIENT_LOGGER";
private static final Logger LOG = LoggerFactory.getLogger(AlluxioLogServerProcess.class); private static final Logger LOG = LoggerFactory.getLogger(AlluxioLogServerProcess.class);
private static final long THREAD_KEEP_ALIVE_TIME_MS = 60000; private static final long THREAD_KEEP_ALIVE_TIME_MS = 60000;
private static final int BASE_SLEEP_TIME_MS = 50;
private static final int MAX_SLEEP_TIME_MS = 30000;
private static final int MAX_NUM_RETRY = 20;


private final String mBaseLogsDir; private final String mBaseLogsDir;
private int mPort; private int mPort;
Expand Down Expand Up @@ -107,7 +104,7 @@ public void start() throws Exception {
continue; continue;
} }
InetAddress inetAddress = client.getInetAddress(); InetAddress inetAddress = client.getInetAddress();
AlluxioLog4jSocketNode clientSocketNode = new AlluxioLog4jSocketNode(this, client); AlluxioLog4jSocketNode clientSocketNode = new AlluxioLog4jSocketNode(mBaseLogsDir, client);
while (true) { while (true) {
try { try {
mThreadPool.execute(clientSocketNode); mThreadPool.execute(clientSocketNode);
Expand Down Expand Up @@ -170,11 +167,4 @@ public Boolean apply(Void input) {
} }
}, WaitForOptions.defaults().setTimeoutMs(10000)); }, WaitForOptions.defaults().setTimeoutMs(10000));
} }

/**
* @return the string representation of the path to base logs directory
*/
public final String getBaseLogsDir() {
return mBaseLogsDir;
}
} }

0 comments on commit 878c3b1

Please sign in to comment.