This is an automated email from the ASF dual-hosted git repository.
jlli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new fb9c1dc Clarify all methods in PinotFS (#3836)
fb9c1dc is described below
commit fb9c1dcded8b3b2f55d8735fa301a190fe4f6ad6
Author: Jialiang Li <[email protected]>
AuthorDate: Thu Feb 14 09:51:54 2019 -0800
Clarify all methods in PinotFS (#3836)
---
.../helix/core/SegmentDeletionManager.java | 14 ++--
.../java/org/apache/pinot/filesystem/PinotFS.java | 75 ++++++++++++----------
2 files changed, 47 insertions(+), 42 deletions(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java
index 2602a3b..adb6cae 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java
@@ -218,13 +218,13 @@ public class SegmentDeletionManager {
URI deletedDirURI =
ControllerConf.getUriFromPath(StringUtil.join(File.separator, _dataDir,
DELETED_SEGMENTS));
PinotFS pinotFS = PinotFSFactory.create(dataDirURI.getScheme());
- // Check that the directory for deleted segments exists
- if (!pinotFS.isDirectory(deletedDirURI)) {
- LOGGER.warn("Deleted segment directory {} does not exist or it is not
directory.", deletedDirURI.toString());
- return;
- }
-
try {
+ // Check that the directory for deleted segments exists.
+ if (!pinotFS.isDirectory(deletedDirURI)) {
+ LOGGER.warn("Deleted segment directory {} does not exist or it is
not directory.", deletedDirURI.toString());
+ return;
+ }
+
String[] tableNameDirs = pinotFS.listFiles(deletedDirURI, false);
if (tableNameDirs == null) {
LOGGER.warn("Deleted segment directory {} does not exist.",
deletedDirURI.toString());
@@ -256,7 +256,7 @@ public class SegmentDeletionManager {
}
}
} catch (IOException e) {
- LOGGER.error("Had trouble deleting directories: {}",
deletedDirURI.toString(), e.toString());
+ LOGGER.error("Had trouble deleting directories: {}",
deletedDirURI.toString(), e);
}
} else {
LOGGER.info("dataDir is not configured, won't delete any expired
segments from deleted directory.");
diff --git
a/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
b/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
index bb9e323..0ed9d56 100644
--- a/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
+++ b/pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
@@ -38,6 +38,8 @@ public abstract class PinotFS implements Closeable {
/**
* Creates a new directory. If parent directories are not created, it will
create them.
+ * If the directory exists, it will return true without doing anything.
+ * @throws IOException on IO failure
*/
public abstract boolean mkdir(URI uri)
throws IOException;
@@ -45,42 +47,46 @@ public abstract class PinotFS implements Closeable {
/**
* Deletes the file at the location provided. If the segmentUri is a
directory, it will delete the entire directory.
* @param segmentUri URI of the segment
- * @param forceDelete true if we want the uri and any suburis to always be
deleted, false if we want delete to fail
+ * @param forceDelete true if we want the uri and any sub-uris to always be
deleted, false if we want delete to fail
* when we want to delete a directory and that directory
is not empty
* @return true if delete is successful else false
- * @throws IOException IO failure
- * @throws Exception if segmentUri is not valid or present
+ * @throws IOException on IO failure, e.g Uri is not present or not valid
*/
public abstract boolean delete(URI segmentUri, boolean forceDelete)
throws IOException;
/**
- * Moves the file from the src to dst. Does not keep the original file. If
the dst has parent directories
- * that haven't been created, this method will create all the necessary
parent directories. If dst already exists,
- * it will overwrite. Will work either for moving a directory or a file.
Currently assumes that both the srcUri
- * and the dstUri are of the same type (both files or both directories). If
srcUri is a file, it will move the file to
- * dstUri's location. If srcUri is a directory, it will move the directory
to dstUri. Does not support moving a file under a
- * directory.
- *
- * For example, if a/b/c is moved to x/y/z, in the case of overwrite, a/b/c
will be renamed to x/y/z.
+ * Moves the file or directory from the src to dst. Does not keep the
original file. If the dst has parent directories
+ * that haven't been created, this method will create all the necessary
parent directories.
+ * If both src and dst are files, dst will be overwritten.
+ * If src is a file and dst is a directory, src file will get moved under
dst directory.
+ * If both src and dst are directories, src directory will get moved under
dst directory.
+ * If src is a directory and dst is a file, operation will fail.
+ * For example, if a file /a/b/c is moved to a file /x/y/z, in the case of
overwrite, the directory /a/b still exists,
+ * but will not contain the file 'c'. Instead, /x/y/z will contain the
contents of 'c'.
+ * If a file /a is moved to a directory /x/y, all the original files under
/x/y will be kept.
* @param srcUri URI of the original file
* @param dstUri URI of the final file location
* @param overwrite true if we want to overwrite the dstURI, false otherwise
* @return true if move is successful
- * @throws IOException on failure
- * @throws Exception if srcUri is not present or valid
+ * @throws IOException on IO failure
*/
public abstract boolean move(URI srcUri, URI dstUri, boolean overwrite)
throws IOException;
/**
- * Same as move except the srcUri is not retained. For example, if x/y/z is
copied to a/b/c, x/y/z will be retained
- * and x/y/z will also be present as a/b/c
+ * Copies the file or directory from the src to dst. The original file is
retained. If the dst has parent directories
+ * that haven't been created, this method will create all the necessary
parent directories.
+ * If both src and dst are files, dst will be overwritten.
+ * If src is a file and dst is a directory, src file will get copied under
dst directory.
+ * If both src and dst are directories, src directory will get copied under
dst directory.
+ * If src is a directory and dst is a file, operation will fail.
+ * For example, if a file /x/y/z is copied to /a/b/c, /x/y/z will be
retained and /x/y/z will also be present as /a/b/c;
+ * if a file /a is copied to a directory /x/y, all the original files under
/x/y will be kept.
* @param srcUri URI of the original file
* @param dstUri URI of the final file location
* @return true if copy is successful
- * @throws IOException on IO Failure
- * @throws Exception if srcUri is not present or valid
+ * @throws IOException on IO failure
*/
public abstract boolean copy(URI srcUri, URI dstUri)
throws IOException;
@@ -89,30 +95,28 @@ public abstract class PinotFS implements Closeable {
* Checks whether the file or directory at the provided location exists.
* @param fileUri URI of file
* @return true if path exists
- * @throws IOException on IO Failure
+ * @throws IOException on IO failure
*/
public abstract boolean exists(URI fileUri)
throws IOException;
/**
- * Returns the length of the file at the provided location. Will throw
exception if a directory
+ * Returns the length of the file at the provided location.
* @param fileUri location of file
* @return the number of bytes
- * @throws IOException on IO Failure
- * @throws Exception if fileUri is not valid or present
+ * @throws IOException on IO failure, e.g if it's a directory.
*/
public abstract long length(URI fileUri)
throws IOException;
/**
- * Lists all the files at the location provided. Lists recursively.
- * Throws exception if this abstract pathname is not valid, or if
- * an I/O error occurs.
+ * Lists all the files and directories at the location provided.
+ * Lists recursively if {@code recursive} is set to true.
+ * Throws IOException if this abstract pathname is not valid, or if an I/O
error occurs.
* @param fileUri location of file
* @param recursive if we want to list files recursively
* @return an array of strings that contains file paths
- * @throws IOException see specific implementation
- * @throws Exception if fileUri is not valid or present
+ * @throws IOException on IO failure. See specific implementation
*/
public abstract String[] listFiles(URI fileUri, boolean recursive)
throws IOException;
@@ -121,7 +125,7 @@ public abstract class PinotFS implements Closeable {
* Copies a file from a remote filesystem to the local one. Keeps the
original file.
* @param srcUri location of current file on remote filesystem
* @param dstFile location of destination on local filesystem
- * @throws Exception if srcUri is not valid or present
+ * @throws Exception if srcUri is not valid or not present, or timeout when
downloading file to local
*/
public abstract void copyToLocalFile(URI srcUri, File dstFile)
throws Exception;
@@ -131,8 +135,7 @@ public abstract class PinotFS implements Closeable {
* afterwards.
* @param srcFile location of src file on local disk
* @param dstUri location of dst on remote filesystem
- * @throws IOException for IO Error
- * @throws Exception if fileUri is not valid or present
+ * @throws Exception if fileUri is not valid or not present, or timeout when
uploading file from local
*/
public abstract void copyFromLocalFile(File srcFile, URI dstUri)
throws Exception;
@@ -141,24 +144,26 @@ public abstract class PinotFS implements Closeable {
* Allows us the ability to determine whether the uri is a directory.
* @param uri location of file or directory
* @return true if uri is a directory, false otherwise.
- * @throws Exception if uri is not valid or present
+ * @throws IOException on IO failure, e.g uri is not valid or not present
*/
- public abstract boolean isDirectory(URI uri);
+ public abstract boolean isDirectory(URI uri)
+ throws IOException;
/**
* Returns the age of the file
* @param uri location of file or directory
* @return A long value representing the time the file was last modified,
measured in milliseconds since epoch
* (00:00:00 GMT, January 1, 1970) or 0L if the file does not exist or if an
I/O error occurs
- * @throws Exception if uri is not valid or present
+ * @throws IOException if uri is not valid or not present
*/
- public abstract long lastModified(URI uri);
+ public abstract long lastModified(URI uri)
+ throws IOException;
/**
* Updates the last modified time of an existing file or directory to be
current time. If the file system object
* does not exist, creates an empty file.
* @param uri location of file or directory
- * @throws IOException if the parent directory doesn't exist.
+ * @throws IOException if the parent directory doesn't exist
*/
public abstract boolean touch(URI uri)
throws IOException;
@@ -166,7 +171,7 @@ public abstract class PinotFS implements Closeable {
/**
* For certain filesystems, we may need to close the filesystem and do
relevant operations to prevent leaks.
* By default, this method does nothing.
- * @throws IOException
+ * @throws IOException on IO failure
*/
public void close()
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
