Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log file fix, MasterLogReader fix, test cases #24

Merged
merged 13 commits into from
Apr 25, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/tachyon
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ else
fi
bin=`cd "$( dirname "$0" )"; pwd`
. "$bin/tachyon-config.sh"
java -cp $TACHYON_JAR $CLASS $@
java -cp $TACHYON_JAR -Dtachyon.home=$TACHYON_HOME -Dlog4j.configuration=file:$TACHYON_HOME/conf/log4j.properties -Dtachyon.logger.type=USER_LOGGER $CLASS $@
14 changes: 12 additions & 2 deletions conf/log4j.properties
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ log4j.appender.Console.layout.ConversionPattern=%d{ISO8601} %-5p %c{1} (%F:%M) -

# Appender for Master
log4j.appender.MASTER_LOGGER=tachyon.Log4jFileAppender
log4j.appender.MASTER_LOGGER.File=logs/master.log
log4j.appender.MASTER_LOGGER.File=${tachyon.home}/logs/master.log

log4j.appender.MASTER_LOGGER.MaxFileSize=10
log4j.appender.MASTER_LOGGER.MaxBackupIndex=100
Expand All @@ -21,11 +21,21 @@ log4j.appender.MASTER_LOGGER.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F

# Appender for Workers
log4j.appender.WORKER_LOGGER=tachyon.Log4jFileAppender
log4j.appender.WORKER_LOGGER.File=logs/worker.log
log4j.appender.WORKER_LOGGER.File=${tachyon.home}/logs/worker.log
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I tested it running from the bin directory and the top level directory


log4j.appender.WORKER_LOGGER.MaxFileSize=10
log4j.appender.WORKER_LOGGER.MaxBackupIndex=100
log4j.appender.WORKER_LOGGER.DeletionPercentage=10
log4j.appender.WORKER_LOGGER.layout=org.apache.log4j.PatternLayout
log4j.appender.WORKER_LOGGER.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F:%M) - %m%n
#log4j.appender.WORKER_LOGGER.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F:%M(%L)) - %m%n

# Appender for User
log4j.appender.USER_LOGGER=tachyon.Log4jFileAppender
log4j.appender.USER_LOGGER.File=${tachyon.home}/logs/user.log
log4j.appender.USER_LOGGER.MaxFileSize=10
log4j.appender.USER_LOGGER.MaxBackupIndex=10
log4j.appender.USER_LOGGER.DeletionPercentage=20
log4j.appender.USER_LOGGER.layout=org.apache.log4j.PatternLayout
log4j.appender.USER_LOGGER.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F:%M) - %m%n
#log4j.appender.USER_LOGGER.layout.ConversionPattern=%d{ISO8601} %-5p %c{2} (%F:%M(%L)) - %m%n
39 changes: 14 additions & 25 deletions src/main/java/tachyon/Log4jFileAppender.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,14 @@ public synchronized void subAppend(LoggingEvent event) {
private String getNewLogFileName(String fileName) {
if (!fileName.isEmpty()) {
String newFileName = "";
int dotIndex = fileName.indexOf(".");
if (dotIndex != -1 && fileName.indexOf("-") == -1) {
String baseName = fileName.substring(0, dotIndex);
String suffix = fileName.substring(dotIndex, fileName.length());
String address = "";
try {
address = "@" + InetAddress.getLocalHost().getHostAddress();
} catch (UnknownHostException uhe) {
address = "@UnknownHost";
}
newFileName = baseName + address + "_"
+ CommonUtils.convertMsToSimpleDate(System.currentTimeMillis()) + suffix;
String address = "";
try {
address = "@" + InetAddress.getLocalHost().getHostAddress();
} catch (UnknownHostException uhe) {
address = "@UnknownHost";
}
newFileName = fileName + address + "_"
+ CommonUtils.convertMsToSimpleDate(System.currentTimeMillis());
File file = new File(newFileName);
if (file.exists()) {
rotateLogs(newFileName);
Expand All @@ -166,12 +161,6 @@ private String getNewLogFileName(String fileName) {
* @param fileName The fileName of the new current log.
*/
private void rotateLogs(String fileName) {
String suffix = "";
if (fileName.indexOf(".") != -1) {
suffix = fileName.substring(fileName.lastIndexOf("."), fileName.length());
}
String prefix = fileName.substring(0, fileName.length() - suffix.length());

if (mCurrentFileBackupIndex == -1) {
int lo = 0;
int hi = mMaxBackupIndex;
Expand All @@ -181,15 +170,15 @@ private void rotateLogs(String fileName) {
mCurrentFileBackupIndex = 1;
break;
}
if (new File(prefix + "_" + mid + suffix).exists()) {
if (new File(prefix + "_" + (mid+1) + suffix).exists()) {
if (new File(fileName + "_" + mid).exists()) {
if (new File(fileName + "_" + (mid+1)).exists()) {
lo = mid;
} else {
mCurrentFileBackupIndex = mid + 1;
break;
}
} else {
if (new File(prefix + "_" + (mid-1) + suffix).exists()) {
if (new File(fileName + "_" + (mid-1)).exists()) {
mCurrentFileBackupIndex = mid;
break;
} else {
Expand All @@ -203,15 +192,15 @@ private void rotateLogs(String fileName) {
if (mCurrentFileBackupIndex >= mMaxBackupIndex) {
int deleteToIndex = (int) Math.ceil(mMaxBackupIndex*mDeletionPercentage/100.0);
for (int i = 1; i < deleteToIndex; i ++) {
new File(prefix + "_" + i + suffix).delete();
new File(fileName + "_" + i).delete();
}
for (int i = deleteToIndex + 1; i <= mMaxBackupIndex; i ++) {
new File(prefix + "_" + i + suffix).renameTo(
new File(prefix + "_" + (i - deleteToIndex) + suffix));
new File(fileName + "_" + i).renameTo(
new File(fileName + "_" + (i - deleteToIndex)));
}
mCurrentFileBackupIndex = mCurrentFileBackupIndex - deleteToIndex;
}
oldFile.renameTo(new File(prefix + "_" + mCurrentFileBackupIndex + suffix));
oldFile.renameTo(new File(fileName + "_" + mCurrentFileBackupIndex));
mCurrentFileBackupIndex ++;
}
}
2 changes: 1 addition & 1 deletion src/main/java/tachyon/MasterLogReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private Pair<LogType, Object> getNext() {

public Pair<LogType, Object> getNextPair() {
if (mCurrent == null) {
getNext();
mCurrent = getNext();
}

Pair<LogType, Object> ret = mCurrent;
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/tachyon/Pair.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,13 @@ public First getFirst() {
public Second getSecond() {
return mSecond;
}

@Override
public synchronized boolean equals(Object o) {
if (!(o instanceof Pair)) {
return false;
}
return mFirst.equals(((Pair<?, ?>) o).getFirst())
&& mSecond.equals(((Pair<?, ?>) o).getSecond());
}
}
3 changes: 1 addition & 2 deletions src/main/java/tachyon/command/TFsShell.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ public int mkdir(String argv[]) throws InvalidPathException, FileAlreadyExistExc
* @throws InvalidPathException
* @throws TException
*/
public int rm(String argv[])
throws FileDoesNotExistException, InvalidPathException, TException {
public int rm(String argv[]) throws InvalidPathException, TException {
if (argv.length != 2) {
System.out.println("Usage: tfs rm <path>");
return -1;
Expand Down
39 changes: 24 additions & 15 deletions src/main/java/tachyon/command/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,55 @@

import java.net.InetSocketAddress;

import tachyon.Constants;
import tachyon.thrift.InvalidPathException;

/**
* Class for convenience methods used by TFsShell.
*/
public class Utils {
private static final String HEADER = "tachyon://";

/**
* Validates the path, verifying that it contains the header and a hostname:port specified.
* @param path The path to be verified.
* @throws InvalidPathException
*/
public static void validateTachyonPath(String path) {
if (!path.startsWith(HEADER)) {
System.out.println("The path format is " + HEADER + "<MASTERHOST>:<PORT>/<PATH_NAME>");
System.exit(-1);
}
path = path.substring(HEADER.length());
if (!path.contains(":") || !path.contains("/")) {
System.out.println("The path format is " + HEADER + "<MASTERHOST>:<PORT>/<PATH_NAME>");
System.exit(-1);
public static String validateTachyonPath(String path) throws InvalidPathException {
if (path.startsWith(HEADER)) {
if (!path.contains(":")) {
throw new InvalidPathException(
"Invalid Path: " + path + "\n Use tachyon://host:port/ or /file");
} else {
return path;
}
} else {
String HOSTNAME = System.getProperty("tachyon.master.hostname", "localhost");
String PORT = System.getProperty("tachyon.master.port", "" + Constants.DEFAULT_MASTER_PORT);
return HEADER + HOSTNAME + ":" + PORT + path;
}
}

/**
* Removes header and hostname:port information from a path, leaving only the local file path.
* @param path The path to obtain the local path from
* @return The local path in string format
* @throws InvalidPathException
*/
public static String getFilePath(String path) {
validateTachyonPath(path);
public static String getFilePath(String path) throws InvalidPathException {
path = validateTachyonPath(path);
path = path.substring(HEADER.length());
return path.substring(path.indexOf("/"));
String ret = path.substring(path.indexOf("/"));
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, you can use CommonUtils.cleanPath(). But why do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because of what I did above, I can remove both if its unnecessary to support that kind of path

}

/**
* Obtains the InetSocketAddress from a path by parsing the hostname:port portion of the path.
* @param path The path to obtain the InetSocketAddress from.
* @return The InetSocketAddress of the master node.
* @throws InvalidPathException
*/
public static InetSocketAddress getTachyonMasterAddress(String path) {
validateTachyonPath(path);
public static InetSocketAddress getTachyonMasterAddress(String path) throws InvalidPathException {
path = validateTachyonPath(path);
path = path.substring(HEADER.length());
String masterAddress = path.substring(0, path.indexOf("/"));
String masterHost = masterAddress.split(":")[0];
Expand Down
4 changes: 4 additions & 0 deletions src/test/java/tachyon/LocalTachyonCluster.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ public int getMasterPort() {
public int getWorkerPort() {
return mWorkerPort;
}

public String getTachyonHome(){
return mTachyonHome;
}

WorkerServiceHandler getWorkerServiceHandler() {
return mWorker.getWorkerServiceHandler();
Expand Down
43 changes: 43 additions & 0 deletions src/test/java/tachyon/MasterLogWriterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,47 @@ public void appendAndFlushCheckpointTest() throws IOException {
Assert.assertEquals(checkpointInfo, toCheck.getSecond());
Assert.assertFalse(mMasterLogReader.hasNext());
}

@Test
public void largeLogTest() throws IOException {
Inode inode;
CheckpointInfo checkpointInfo;
int numEntries = 100000;
for (int i = 0; i < numEntries; i ++) {
switch (i % 3) {
case 0:
inode = new InodeFile("/testFile" + i, 1 + i, 0);
mMasterLogWriter.append(inode, true);
break;
case 1:
inode = new InodeFolder("/testFolder" + i, 1 + i, 0);
mMasterLogWriter.append(inode, true);
break;
case 2:
checkpointInfo = new CheckpointInfo(i);
mMasterLogWriter.appendAndFlush(checkpointInfo);
break;
}
}
mMasterLogReader = new MasterLogReader(mLogFile);
for (int i = 0; i < numEntries; i ++) {
switch (i % 3) {
case 0:
inode = new InodeFile("/testFile" + i, 1 + i, 0);
Assert.assertEquals(new Pair<LogType, Object>(LogType.InodeFile, inode),
mMasterLogReader.getNextPair());
break;
case 1:
inode = new InodeFolder("/testFolder" + i, 1 + i, 0);
Assert.assertEquals(new Pair<LogType, Object>(LogType.InodeFolder, inode),
mMasterLogReader.getNextPair());
break;
case 2:
checkpointInfo = new CheckpointInfo(i);
Assert.assertEquals(new Pair<LogType, Object>(LogType.CheckpointInfo, checkpointInfo),
mMasterLogReader.getNextPair());
break;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How long does this test run?

Could we remove
List<Pair<LogType, Object>> loggedObjects = new ArrayList<Pair<LogType, Object>>(100000);
and regenerate on the fly when do Assert.assertEquals(pair, mMasterLogReader.getNextPair()); ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I originally had to keep track because of the randomness, but not anymore. It takes around 2 seconds.

}
2 changes: 1 addition & 1 deletion src/test/java/tachyon/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static int createSimpleByteFile(TachyonClient client, String fileName, Op

return fileId;
}

public static byte[] getIncreasingByteArray(int len) {
byte[] ret = new byte[len];
for (int k = 0; k < len; k ++) {
Expand Down
Loading