From 79ef279a38fc9f049997008b1a1a06c418964d32 Mon Sep 17 00:00:00 2001 From: Calvin Jia Date: Mon, 7 Sep 2015 04:30:04 -0700 Subject: [PATCH] Address minor style and documentation comments. --- .../main/java/tachyon/client/InStream.java | 2 +- .../java/tachyon/client/ResourcePool.java | 9 ++++++--- .../tachyon/client/TachyonFSTestUtils.java | 4 ++-- .../client/block/LocalBlockInStream.java | 20 +++++++++---------- .../client/block/LocalBlockOutStream.java | 5 +++-- .../client/block/RemoteBlockInStream.java | 3 ++- .../client/block/RemoteBlockOutStream.java | 3 ++- .../client/block/TachyonBlockStore.java | 2 +- 8 files changed, 27 insertions(+), 21 deletions(-) diff --git a/clients/unshaded/src/main/java/tachyon/client/InStream.java b/clients/unshaded/src/main/java/tachyon/client/InStream.java index 04e1f5608ba3..d86ea05567b7 100644 --- a/clients/unshaded/src/main/java/tachyon/client/InStream.java +++ b/clients/unshaded/src/main/java/tachyon/client/InStream.java @@ -28,7 +28,7 @@ public abstract class InStream extends InputStream { * Moves the starting read position of the stream to the specified position which is relative to * the start of the stream. Seeking to a position before the current read position is supported. * - * @param pos The position to seek to, it must be between 0 and the size of the block. + * @param pos The position to seek to, it must be between 0 and the size of the block - 1. * @throws IOException if the seek fails due to an error accessing the stream at the position */ public abstract void seek(long pos) throws IOException; diff --git a/clients/unshaded/src/main/java/tachyon/client/ResourcePool.java b/clients/unshaded/src/main/java/tachyon/client/ResourcePool.java index 5a908e7af238..a755fbd57502 100644 --- a/clients/unshaded/src/main/java/tachyon/client/ResourcePool.java +++ b/clients/unshaded/src/main/java/tachyon/client/ResourcePool.java @@ -20,15 +20,18 @@ /** * Class representing a pool of resources to be temporarily used and returned. Inheriting classes - * should implement the close method as well as initialize the resources in the constructor. + * should implement the close method as well as initialize the resources in the constructor. The + * implemented methods are thread-safe and inheriting classes should also written in a + * thread-safe manner. See {@link tachyon.client.file.FSMasterClientPool} as an example. * * @param The type of resource this pool manages. */ +// TODO: This may fit better in the common module public abstract class ResourcePool { protected final Object mCapacityLock; - protected int mCurrentCapacity; protected final int mMaxCapacity; - protected BlockingQueue mResources; + protected final BlockingQueue mResources; + protected int mCurrentCapacity; public ResourcePool(int maxCapacity) { mCapacityLock = new Object(); diff --git a/clients/unshaded/src/main/java/tachyon/client/TachyonFSTestUtils.java b/clients/unshaded/src/main/java/tachyon/client/TachyonFSTestUtils.java index 584417ded431..89f122839688 100644 --- a/clients/unshaded/src/main/java/tachyon/client/TachyonFSTestUtils.java +++ b/clients/unshaded/src/main/java/tachyon/client/TachyonFSTestUtils.java @@ -38,8 +38,8 @@ public final class TachyonFSTestUtils { */ public static TachyonFile createByteFile(TachyonFileSystem tfs, String fileName, ClientOptions options, int len) throws IOException { - return createByteFile(tfs, fileName, options.getTachyonStorageType(), options.getUnderStorageType(), - len, options.getBlockSize()); + return createByteFile(tfs, fileName, options.getTachyonStorageType(), + options.getUnderStorageType(), len, options.getBlockSize()); } /** diff --git a/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockInStream.java b/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockInStream.java index 819347cd044d..08e88c7291de 100644 --- a/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockInStream.java +++ b/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockInStream.java @@ -30,7 +30,8 @@ /** * This class provides a streaming API to read a block in Tachyon. The data will be directly read - * from the local machine's storage. + * from the local machine's storage. The instances of this class should only be used by one + * thread and are not thread safe. */ public class LocalBlockInStream extends BlockInStream { private final long mBlockId; @@ -86,7 +87,7 @@ public void close() throws IOException { @Override public int read() throws IOException { - failIfClosed(); + checkIfClosed(); if (mData.remaining() == 0) { close(); return -1; @@ -96,13 +97,13 @@ public int read() throws IOException { @Override public int read(byte[] b) throws IOException { - failIfClosed(); + checkIfClosed(); return read(b, 0, b.length); } @Override public int read(byte[] b, int off, int len) throws IOException { - failIfClosed(); + checkIfClosed(); Preconditions.checkArgument(b != null, "Buffer is null"); Preconditions.checkArgument(off >= 0 && len >= 0 && len + off <= b.length, String .format("Buffer length (%d), offset(%d), len(%d)", b.length, off, len)); @@ -124,8 +125,9 @@ public long remaining() { return mData.remaining(); } + @Override public void seek(long pos) throws IOException { - failIfClosed(); + checkIfClosed(); Preconditions.checkArgument(pos >= 0, "Seek position is negative: " + pos); Preconditions.checkArgument(pos <= mData.limit(), "Seek position is past buffer limit: " + pos + ", Buffer Size = " + mData.limit()); @@ -134,7 +136,7 @@ public void seek(long pos) throws IOException { @Override public long skip(long n) throws IOException { - failIfClosed(); + checkIfClosed(); if (n <= 0) { return 0; } @@ -147,9 +149,7 @@ public long skip(long n) throws IOException { return ret; } - private void failIfClosed() throws IOException { - if (mClosed) { - throw new IOException("Cannot do operations on a closed BlockInStream"); - } + private void checkIfClosed() throws IOException { + Preconditions.checkState(!mClosed, "Cannot do operations on a closed BlockInStream"); } } diff --git a/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockOutStream.java b/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockOutStream.java index c3d8a8603aca..9db9d3ff3ee8 100644 --- a/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockOutStream.java +++ b/clients/unshaded/src/main/java/tachyon/client/block/LocalBlockOutStream.java @@ -32,8 +32,9 @@ import tachyon.worker.WorkerClient; /** - * Provides a streaming API to write to a Tachyon block. This output stream will directly write - * the input to a file in local Tachyon storage. + * Provides a streaming API to write to a Tachyon block. This output stream will directly write the + * input to a file in local Tachyon storage. The instances of this class should only be used by one + * thread and are not thread safe. */ public class LocalBlockOutStream extends BufferedBlockOutStream { private final Closer mCloser; diff --git a/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockInStream.java b/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockInStream.java index 5d0b85ff0a84..044a25867f94 100644 --- a/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockInStream.java +++ b/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockInStream.java @@ -28,7 +28,8 @@ /** * This class provides a streaming API to read a block in Tachyon. The data will be transferred - * through a Tachyon worker's dataserver to the client. + * through a Tachyon worker's dataserver to the client. The instances of this class should only be + * used by one thread and are not thread safe. */ public class RemoteBlockInStream extends BlockInStream { private final long mBlockId; diff --git a/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockOutStream.java b/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockOutStream.java index f5920db597bf..98f8f0f3a307 100644 --- a/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockOutStream.java +++ b/clients/unshaded/src/main/java/tachyon/client/block/RemoteBlockOutStream.java @@ -23,7 +23,8 @@ /** * Provides a streaming API to write to a Tachyon block. This output stream will send the write - * through a Tachyon worker which will then write the block to a file in Tachyon storage. + * through a Tachyon worker which will then write the block to a file in Tachyon storage. The + * instances of this class should only be used by one thread and are not thread safe. */ public final class RemoteBlockOutStream extends BufferedBlockOutStream { private final RemoteBlockWriter mRemoteWriter; diff --git a/clients/unshaded/src/main/java/tachyon/client/block/TachyonBlockStore.java b/clients/unshaded/src/main/java/tachyon/client/block/TachyonBlockStore.java index 80915077eb73..85f15dafffe4 100644 --- a/clients/unshaded/src/main/java/tachyon/client/block/TachyonBlockStore.java +++ b/clients/unshaded/src/main/java/tachyon/client/block/TachyonBlockStore.java @@ -52,7 +52,7 @@ public static synchronized TachyonBlockStore get() { /** * Creates a Tachyon block store. */ - public TachyonBlockStore() { + private TachyonBlockStore() { mContext = BSContext.INSTANCE; }