Skip to content

HADOOP-11452 make rename/3 public #743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ boolean apply(Path p) throws IOException {
}

/**
* Rename files/dirs
* Rename files/dirs.
*/
@Override
@SuppressWarnings("deprecation")
Expand Down Expand Up @@ -672,6 +672,51 @@ public boolean rename(Path src, Path dst) throws IOException {
}
}

/**
* Rename files/dirs.
* <p></p>
* If the rename of the inner FS does not fail by raising
* an exception, any checksum file for the source is copied.
* If the source had no checksum, any checksum for the destination
* is deleted, so that there is no inconsistent checksum
* for the now-renamed file.
* <p></p>
* @param source source file/dir
* @param dest destination file/dir/path
* @param options options for the renaming of the source file.
* These are not used for renaming the checksum, which always uses
* {@link Options.Rename#OVERWRITE}.Are
* @throws IOException
*/
@Override
public void rename(final Path source,
final Path dest,
final Options.Rename... options)
throws IOException {

// invoke rename on the wrapped FS.
// This will raise an exception on any failure.
fs.rename(source, dest, options);

// if the application gets this far, the rename
// succeeded. Therefore the checksum should
// be renamed too,
Path srcCheckFile = getChecksumFile(source);
Path dstCheckFile = getChecksumFile(dest);
if (fs.exists(srcCheckFile)) {
// try to rename checksum
// the OVERWRITE option is always used; there's no attempt
// to merge in any options supplied for merging the
// main file.
// If new options are added, this decision may need to
// be revisited.
fs.rename(srcCheckFile, dstCheckFile, Options.Rename.OVERWRITE);
} else {
// no src checksum, so remove dest checksum if present
fs.delete(dstCheckFile, true);
}
}

/**
* Implement the delete(Path, boolean) in checksum
* file system.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@
public class FSExceptionMessages {

/**
* The operation failed because the stream is closed: {@value}
* The operation failed because the stream is closed: {@value}.
*/
public static final String STREAM_IS_CLOSED = "Stream is closed!";

/**
* Negative offset seek forbidden : {@value}
* Negative offset seek forbidden : {@value}.
*/
public static final String NEGATIVE_SEEK =
"Cannot seek to a negative offset";

/**
* Seeks : {@value}
* Seeks : {@value}.
*/
public static final String CANNOT_SEEK_PAST_EOF =
"Attempted to seek or read past the end of the file";
Expand All @@ -51,4 +51,101 @@ public class FSExceptionMessages {

public static final String PERMISSION_DENIED_BY_STICKY_BIT =
"Permission denied by sticky bit";

/**
* Renaming a destination under source is forbidden.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_UNDER_SOURCE =
"Rename destination %s is a directory or file under source %s";

/**
* Renaming a destination to source is forbidden.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_EQUALS_SOURCE =
"The source %s and destination %s are the same";

/**
* Renaming to root is forbidden.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_IS_ROOT =
"Rename destination cannot be the root path \"/\"";

/**
* The parent of a rename destination is not found.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_NO_PARENT_OF =
"Rename destination parent of %s not found";

/**
* The parent of a rename destination is not found.
* This is a format string, taking the parent path of the destination
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_NO_PARENT =
"Rename destination parent %s not found";

/**
* The parent of a rename destination is not a directory.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_PARENT_NOT_DIRECTORY =
"Rename destination parent %s is a file";

/**
* The rename destination is not an empty directory.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_NOT_EMPTY =
"Rename destination directory is not empty: %s";

/**
* The rename destination is not an empty directory.
* <p></p>
* {@value}.
*/
public static final String RENAME_DEST_EXISTS =
"Rename destination %s already exists";

/**
* The rename source doesn't exist.
* <p></p>
* {@value}.
*/
public static final String RENAME_SOURCE_NOT_FOUND =
"Rename source %s is not found";

/**
* The rename source and dest are of different types.
* <p></p>
* {@value}.
*/
public static final String RENAME_SOURCE_DEST_DIFFERENT_TYPE =
"Rename source %s and destination %s are of different types";

/**
* The rename source doesn't exist.
* <p></p>
* {@value}.
*/
public static final String RENAME_SOURCE_IS_ROOT =
"Rename source cannot be the root";

/**
* The rename failed for an unknown reason.
* <p></p>
* {@value}.
*/
public static final String RENAME_FAILED = "Rename from %s to %s failed";

}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.apache.hadoop.fs.impl.AbstractFSBuilderImpl;
import org.apache.hadoop.fs.impl.FutureDataInputStreamBuilderImpl;
import org.apache.hadoop.fs.impl.OpenFileParameters;
import org.apache.hadoop.fs.impl.FileSystemRename3Action;
import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.AclStatus;
import org.apache.hadoop.fs.permission.FsAction;
Expand Down Expand Up @@ -1519,94 +1520,58 @@ public boolean setReplication(Path src, short replication)
public abstract boolean rename(Path src, Path dst) throws IOException;

/**
* Renames Path src to Path dst
* Renames Path source to Path dest.
* <ul>
* <li>Fails if src is a file and dst is a directory.</li>
* <li>Fails if src is a directory and dst is a file.</li>
* <li>Fails if the parent of dst does not exist or is a file.</li>
* <li>Fails if source is a file and dest is a directory.</li>
* <li>Fails if source is a directory and dest is a file.</li>
* <li>Fails if the parent of dest does not exist or is a file.</li>
* </ul>
* <p>
* If OVERWRITE option is not passed as an argument, rename fails
* if the dst already exists.
* if the dest already exists.
* <p>
* If OVERWRITE option is passed as an argument, rename overwrites
* the dst if it is a file or an empty directory. Rename fails if dst is
* the dest if it is a file or an empty directory. Rename fails if dest is
* a non-empty directory.
* <p>
* Note that atomicity of rename is dependent on the file system
* implementation. Please refer to the file system documentation for
* details. This default implementation is non atomic.
* <p>
* This method is deprecated since it is a temporary method added to
* support the transition from FileSystem to FileContext for user
* applications.
*
* @param src path to be renamed
* @param dst new path after rename
* @throws FileNotFoundException src path does not exist, or the parent
* path of dst does not exist.
* @param source path to be renamed
* @param dest new path after rename
* @param options rename options.
* @throws FileNotFoundException source path does not exist, or the parent
* path of dest does not exist.
* @throws FileAlreadyExistsException dest path exists and is a file
* @throws ParentNotDirectoryException if the parent path of dest is not
* a directory
* @throws IOException on failure
*/
@Deprecated
Copy link

@bgaborg bgaborg Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You remove the deprecation here from the abstract FS. How will it affect the target release it can get in?
FS is

@InterfaceAudience.Public
@InterfaceStability.Stable

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it changed to protected to public. The same question applies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's exactly the purpose: take a method which is protected and used only by FileContext, and make it public. It's been in for a while, been stable -and now needs tests and documentation of what it really does

protected void rename(final Path src, final Path dst,
public void rename(final Path source, final Path dest,
final Rename... options) throws IOException {
final Path sourcePath = makeQualified(source);
final Path destPath = makeQualified(dest);
// Default implementation
final FileStatus srcStatus = getFileLinkStatus(src);
if (srcStatus == null) {
throw new FileNotFoundException("rename source " + src + " not found.");
}

boolean overwrite = false;
if (null != options) {
for (Rename option : options) {
if (option == Rename.OVERWRITE) {
overwrite = true;
}
}
}
new FileSystemRename3Action()
.rename(
new FileSystemRename3Action.RenameValidationBuilder()
.withSourcePath(sourcePath)
.withDestPath(destPath)
.withRenameCallbacks(
createRenameCallbacks())
.withRenameOptions(options)
.build());
}

FileStatus dstStatus;
try {
dstStatus = getFileLinkStatus(dst);
} catch (IOException e) {
dstStatus = null;
}
if (dstStatus != null) {
if (srcStatus.isDirectory() != dstStatus.isDirectory()) {
throw new IOException("Source " + src + " Destination " + dst
+ " both should be either file or directory");
}
if (!overwrite) {
throw new FileAlreadyExistsException("rename destination " + dst
+ " already exists.");
}
// Delete the destination that is a file or an empty directory
if (dstStatus.isDirectory()) {
FileStatus[] list = listStatus(dst);
if (list != null && list.length != 0) {
throw new IOException(
"rename cannot overwrite non empty destination directory " + dst);
}
}
delete(dst, false);
} else {
final Path parent = dst.getParent();
final FileStatus parentStatus = getFileStatus(parent);
if (parentStatus == null) {
throw new FileNotFoundException("rename destination parent " + parent
+ " not found.");
}
if (!parentStatus.isDirectory()) {
throw new ParentNotDirectoryException("rename destination parent " + parent
+ " is a file.");
}
}
if (!rename(src, dst)) {
throw new IOException("rename from " + src + " to " + dst + " failed.");
}
/**
* Override point, create any custom rename callbacks.
* This only needs to be overridden if the subclass has
* optimized operations.
* @return callbacks for renaming.
*/
protected FileSystemRename3Action.RenameCallbacks createRenameCallbacks() {
return FileSystemRename3Action.callbacksFromFileSystem(this);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.impl.FileSystemRename3Action;
import org.apache.hadoop.fs.impl.OpenFileParameters;
import org.apache.hadoop.fs.permission.AclEntry;
import org.apache.hadoop.fs.permission.AclStatus;
Expand Down Expand Up @@ -252,7 +253,7 @@ public boolean rename(Path src, Path dst) throws IOException {
}

@Override
protected void rename(Path src, Path dst, Rename... options)
public void rename(Path src, Path dst, Rename... options)
throws IOException {
fs.rename(src, dst, options);
}
Expand Down Expand Up @@ -742,4 +743,8 @@ public boolean hasPathCapability(final Path path, final String capability)
}
}

@Override
protected FileSystemRename3Action.RenameCallbacks createRenameCallbacks() {
return fs.createRenameCallbacks();
}
}
Loading