You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by jl...@apache.org on 2019/02/14 17:51:59 UTC
[incubator-pinot] branch master updated: Clarify all methods in
PinotFS (#3836)
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 <jl...@linkedin.com>
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: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org