cloud-fan commented on a change in pull request #32881:
URL: https://github.com/apache/spark/pull/32881#discussion_r655190329
##########
File path:
core/src/main/scala/org/apache/spark/internal/io/FileCommitProtocol.scala
##########
@@ -73,35 +73,31 @@ abstract class FileCommitProtocol extends Logging {
* Notifies the commit protocol to add a new file, and gets back the full
path that should be
* used. Must be called on the executors when running tasks.
*
- * Note that the returned temp file may have an arbitrary path. The commit
protocol only
- * promises that the file will be at the location specified by the arguments
after job commit.
+ * Note that "relativePath" parameter specifies the relative path of
returned temp file. The full
+ * path is left to the commit protocol to decide. The commit protocol only
promises that the file
+ * will be at the location specified by the relative path after job commits.
*
- * A full file path consists of the following parts:
- * 1. the base path
- * 2. some sub-directory within the base path, used to specify partitioning
- * 3. file prefix, usually some unique job id with the task id
- * 4. bucket id
- * 5. source specific file extension, e.g. ".snappy.parquet"
- *
- * The "dir" parameter specifies 2, and "ext" parameter specifies both 4 and
5, and the rest
- * are left to the commit protocol implementation to decide.
- *
- * Important: it is the caller's responsibility to add uniquely identifying
content to "ext"
- * if a task is going to write out multiple files to the same dir. The file
commit protocol only
- * guarantees that files written by different tasks will not conflict.
+ * Important: it is the caller's responsibility to add uniquely identifying
content to
+ * "relativePath" if a task is going to write out multiple files to the same
directory. The file
+ * commit protocol only guarantees that files written by different tasks
will not conflict.
*/
- def newTaskTempFile(taskContext: TaskAttemptContext, dir: Option[String],
ext: String): String
+ def newTaskTempFile(taskContext: TaskAttemptContext, relativePath: String):
String
/**
* Similar to newTaskTempFile(), but allows files to committed to an
absolute output location.
* Depending on the implementation, there may be weaker guarantees around
adding files this way.
*
- * Important: it is the caller's responsibility to add uniquely identifying
content to "ext"
- * if a task is going to write out multiple files to the same dir. The file
commit protocol only
- * guarantees that files written by different tasks will not conflict.
+ * "relativePath" parameter specifies the relative path of returned temp
file, and "finalPath"
+ * parameter specifies the full path of file after job commit. The commit
protocol promises that
+ * the file will be at the location specified by the "finalPath" after job
commits.
+ *
+ * Important: it is the caller's responsibility to add uniquely identifying
content to
+ * "relativePath" and "finalPath" if a task is going to write out multiple
files to the same
+ * directory. The file commit protocol only guarantees that files written by
different tasks will
+ * not conflict.
*/
def newTaskTempFileAbsPath(
- taskContext: TaskAttemptContext, absoluteDir: String, ext: String):
String
+ taskContext: TaskAttemptContext, relativePath: String, finalPath:
String): String
Review comment:
note: I can see that, this API makes the code simpler, but it makes the
semantic a bit more complicated. What if the final path doesn't have the same
file name as the `relativePath`? Maybe it's better to have `fileName: String,
targetDir: String`. Then the semantic is pretty here: the impl should commit
the new file to the target dir.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
