Skip to content

Commit

Permalink
[TACHYON-1061] Throw detailed exceptions isolated from RPC framework
Browse files Browse the repository at this point in the history
The bulk of this commit is renaming/refactors, so below we detail
the important things to look at

In tachyon.thrift, we deleted all the thrift exceptions, and
replaced them with two exceptions, TachyonTException and
ThriftIOException, which are general containers for Tachyon
exceptions and IOExceptions, respectively. Also modified the
service interfaces to only throw those exceptions.

In tacyhon/exception/, we defined the TachyonException class as a
superclass of all Tachyon exceptions, which can convert back and
forth between TachyonTException using error codes. We then
defined an exception subclass for each of the error code values,
which will be the specific exceptions that we throw around in
Tachyon everywhere except the thrift layer.

Finally, in the client code, whenever we make an RPC call, we
immediately convert the TachyonTException to a TachyonException
and the ThriftIOException to an IOException, and propagate that
up to the top layer of the client. At any user-facing boundary of
the client (like TachyonFileSystem), we'll unwrap the
TachyonException with any specific ones we think it might throw.
Otherwise, we'll throw the general TachyonException as-is, as a
catch-all for exceptions we didn't expect, or might add later. By
including the TachyonException in all the 'throws' clauses of the
function signatures, we allow for backwards compatibility by
ensuring that newly added exceptions won't cause a compile-time
error in user code.
  • Loading branch information
manugoyal committed Oct 1, 2015
1 parent 5f8f63d commit 191095c
Show file tree
Hide file tree
Showing 120 changed files with 19,728 additions and 21,549 deletions.
42 changes: 19 additions & 23 deletions clients/unshaded/src/main/java/tachyon/client/TachyonFS.java
Expand Up @@ -27,7 +27,6 @@
import java.util.concurrent.Executors; import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;


import org.apache.thrift.TException;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;


Expand All @@ -41,15 +40,12 @@
import tachyon.client.file.FileSystemContext; import tachyon.client.file.FileSystemContext;
import tachyon.client.table.RawTable; import tachyon.client.table.RawTable;
import tachyon.conf.TachyonConf; import tachyon.conf.TachyonConf;
import tachyon.thrift.DependencyDoesNotExistException; import tachyon.exception.TachyonException;
import tachyon.thrift.DependencyInfo; import tachyon.thrift.DependencyInfo;
import tachyon.thrift.FileBlockInfo; import tachyon.thrift.FileBlockInfo;
import tachyon.thrift.FileDoesNotExistException;
import tachyon.thrift.FileInfo; import tachyon.thrift.FileInfo;
import tachyon.thrift.InvalidPathException;
import tachyon.thrift.RawTableInfo; import tachyon.thrift.RawTableInfo;
import tachyon.thrift.WorkerInfo; import tachyon.thrift.WorkerInfo;
import tachyon.underfs.UnderFileSystem;
import tachyon.util.ThreadFactoryUtils; import tachyon.util.ThreadFactoryUtils;
import tachyon.util.io.FileUtils; import tachyon.util.io.FileUtils;
import tachyon.util.network.NetworkAddressUtils; import tachyon.util.network.NetworkAddressUtils;
Expand Down Expand Up @@ -276,7 +272,7 @@ public synchronized void close() throws IOException {
synchronized void completeFile(long fid) throws IOException { synchronized void completeFile(long fid) throws IOException {
try { try {
mFSMasterClient.completeFile(fid); mFSMasterClient.completeFile(fid);
} catch (TException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -325,7 +321,7 @@ public synchronized long createFile(TachyonURI path, TachyonURI ufsPath, long bl
} else { } else {
return mFSMasterClient.loadFileInfoFromUfs(path.getPath(), recursive); return mFSMasterClient.loadFileInfoFromUfs(path.getPath(), recursive);
} }
} catch (TException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -382,13 +378,13 @@ public synchronized boolean delete(long fileId, TachyonURI path, boolean recursi
if (fileId == -1) { if (fileId == -1) {
try { try {
fileId = mFSMasterClient.getFileId(path.getPath()); fileId = mFSMasterClient.getFileId(path.getPath());
} catch (InvalidPathException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
try { try {
return mFSMasterClient.deleteFile(fileId, recursive); return mFSMasterClient.deleteFile(fileId, recursive);
} catch (TException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -432,7 +428,7 @@ public synchronized long getBlockId(long fileId, int blockIndex) throws IOExcept


try { try {
return mFSMasterClient.getFileBlockInfo(fileId, blockIndex).blockInfo.getBlockId(); return mFSMasterClient.getFileBlockInfo(fileId, blockIndex).blockInfo.getBlockId();
} catch (TException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -552,7 +548,7 @@ public synchronized List<FileBlockInfo> getFileBlocks(long fid) throws IOExcepti
// TODO(hy) Should add timeout to improve this. // TODO(hy) Should add timeout to improve this.
try { try {
return mFSMasterClient.getFileBlockInfoList(fid); return mFSMasterClient.getFileBlockInfoList(fid);
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -596,7 +592,7 @@ private synchronized <K> FileInfo getFileStatus(Map<K, FileInfo> cache, K key, l
if (fileId == -1) { if (fileId == -1) {
try { try {
fileId = mFSMasterClient.getFileId(path); fileId = mFSMasterClient.getFileId(path);
} catch (InvalidPathException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand All @@ -606,7 +602,7 @@ private synchronized <K> FileInfo getFileStatus(Map<K, FileInfo> cache, K key, l
} }
try { try {
info = mFSMasterClient.getFileInfo(fileId); info = mFSMasterClient.getFileInfo(fileId);
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
path = info.getPath(); path = info.getPath();
Expand Down Expand Up @@ -634,7 +630,7 @@ public synchronized FileInfo getFileStatus(long fileId, TachyonURI path,
if (fileId == -1) { if (fileId == -1) {
try { try {
fileId = mFSMasterClient.getFileId(path.getPath()); fileId = mFSMasterClient.getFileId(path.getPath());
} catch (InvalidPathException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -803,7 +799,7 @@ public synchronized List<FileInfo> listStatus(TachyonURI path) throws IOExceptio
validateUri(path); validateUri(path);
try { try {
return mFSMasterClient.getFileInfoList(getFileStatus(-1, path).getFileId()); return mFSMasterClient.getFileInfoList(getFileStatus(-1, path).getFileId());
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -855,7 +851,7 @@ public synchronized boolean mkdirs(TachyonURI path, boolean recursive) throws IO
validateUri(path); validateUri(path);
try { try {
return mFSMasterClient.createDirectory(path.getPath(), recursive); return mFSMasterClient.createDirectory(path.getPath(), recursive);
} catch (TException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -888,13 +884,13 @@ public synchronized boolean freepath(long fileId, TachyonURI path, boolean recur
if (fileId == -1) { if (fileId == -1) {
try { try {
fileId = mFSMasterClient.getFileId(path.getPath()); fileId = mFSMasterClient.getFileId(path.getPath());
} catch (InvalidPathException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
try { try {
return mFSMasterClient.free(fileId, recursive); return mFSMasterClient.free(fileId, recursive);
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -931,13 +927,13 @@ public synchronized boolean rename(long fileId, TachyonURI srcPath, TachyonURI d
if (fileId == -1) { if (fileId == -1) {
try { try {
fileId = mFSMasterClient.getFileId(srcPath.getPath()); fileId = mFSMasterClient.getFileId(srcPath.getPath());
} catch (InvalidPathException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
try { try {
return mFSMasterClient.renameFile(fileId, dstPath.getPath()); return mFSMasterClient.renameFile(fileId, dstPath.getPath());
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand All @@ -951,7 +947,7 @@ public synchronized boolean rename(long fileId, TachyonURI srcPath, TachyonURI d
public synchronized void reportLostFile(long fileId) throws IOException { public synchronized void reportLostFile(long fileId) throws IOException {
try { try {
mFSMasterClient.reportLostFile(fileId); mFSMasterClient.reportLostFile(fileId);
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand All @@ -965,7 +961,7 @@ public synchronized void reportLostFile(long fileId) throws IOException {
public synchronized void requestFilesInDependency(int depId) throws IOException { public synchronized void requestFilesInDependency(int depId) throws IOException {
try { try {
mFSMasterClient.requestFilesInDependency(depId); mFSMasterClient.requestFilesInDependency(depId);
} catch (DependencyDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down Expand Up @@ -1008,7 +1004,7 @@ public synchronized long requestSpace(long blockId, long requestSpaceBytes) thro
public synchronized void setPinned(long fid, boolean pinned) throws IOException { public synchronized void setPinned(long fid, boolean pinned) throws IOException {
try { try {
mFSMasterClient.setPinned(fid, pinned); mFSMasterClient.setPinned(fid, pinned);
} catch (FileDoesNotExistException e) { } catch (TachyonException e) {
throw new IOException(e); throw new IOException(e);
} }
} }
Expand Down
Expand Up @@ -23,7 +23,6 @@
import tachyon.client.BlockMasterClient; import tachyon.client.BlockMasterClient;
import tachyon.client.ClientContext; import tachyon.client.ClientContext;
import tachyon.resource.ResourcePool; import tachyon.resource.ResourcePool;
import tachyon.conf.TachyonConf;
import tachyon.util.ThreadFactoryUtils; import tachyon.util.ThreadFactoryUtils;


/** /**
Expand Down
Expand Up @@ -25,10 +25,8 @@
import tachyon.Constants; import tachyon.Constants;
import tachyon.client.ClientContext; import tachyon.client.ClientContext;
import tachyon.resource.ResourcePool; import tachyon.resource.ResourcePool;
import tachyon.conf.TachyonConf;
import tachyon.thrift.NetAddress; import tachyon.thrift.NetAddress;
import tachyon.util.ThreadFactoryUtils; import tachyon.util.ThreadFactoryUtils;
import tachyon.worker.ClientMetrics;
import tachyon.worker.WorkerClient; import tachyon.worker.WorkerClient;


/** /**
Expand Down

0 comments on commit 191095c

Please sign in to comment.