Skip to content
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

Allow rename() to overwrite, fix an uncaught ex and reenable UT #18263

Merged
merged 25 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -382,11 +382,12 @@ public void rename(AlluxioURI src, AlluxioURI dst, RenamePOptions options)
} catch (RuntimeException ex) {
if (ex instanceof StatusRuntimeException) {
YichuanSun marked this conversation as resolved.
Show resolved Hide resolved
Status.Code code = ((StatusRuntimeException) ex).getStatus().getCode();
if (code == Status.FAILED_PRECONDITION.getCode()) {
// throw new RuntimeException(ex.getMessage());
} else if (code == Status.NOT_FOUND.getCode()) {
if (Status.FAILED_PRECONDITION.getCode().equals(code)) {
LOG.error(String.format(
YichuanSun marked this conversation as resolved.
Show resolved Hide resolved
"Precondition failed: cannot rename %s to %s", src.toString(), dst.toString()));
} else if (Status.NOT_FOUND.getCode().equals(code)) {
throw new FileNotFoundException(ex.getMessage());
} else if (code == Status.ALREADY_EXISTS.getCode()) {
} else if (Status.ALREADY_EXISTS.getCode().equals(code)) {
// throw exception here, no fallback even the fallback is open
// which means even ufs support overwrite, alluxio won't allow doing it
throw new FileAlreadyExistsException(ex.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ public boolean rename(Path src, Path dst) throws IOException {
if (mStatistics != null) {
mStatistics.incrementWriteOps(1);
}
if (src == dst) {
if (src.equals(dst)) {
LOG.debug("Renaming {} to the same location", src);
return true;
YichuanSun marked this conversation as resolved.
Show resolved Hide resolved
}
Expand All @@ -720,25 +720,26 @@ public boolean rename(Path src, Path dst) throws IOException {
try {
boolean res = tryMoveIntoDirectory(src, dst, srcPath, dstPath);
if (!res) {
LOG.warn("Failed to rename file and dst {} is not a directory", dst, exToLog);
LOG.warn("Failed to rename src {} to dst {}"
+ "because the dst is not a directory", src, dst, exToLog);
}
} catch (IOException | AlluxioException | AlluxioRuntimeException e2) {
LOG.error("Failed to rename file {} and failed to move into dst directory {}",
LOG.error("Failed to rename src {} to dst {} with exception {},"
+ "and fail to move src to dst as a folder with exception",
src, dst, exToLog, e2);
return false;
}
} catch (IOException e) {
} catch (IOException | RuntimeException e) {
LOG.error("Failed to rename {} to {}", src, dst, e);
return false;
} catch (RuntimeException e) {
return false;
}
return true;
}

/**
* rename when previous rename fail, reconstruct path and check status.
* In order to support hdfs rename interface
*
* @param src src
* @param dst dst
* @param srcPath srcPath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public final class FileSystemRenameIntegrationTest extends BaseIntegrationTest {
.setProperty(PropertyKey.USER_FILE_WRITE_TYPE_DEFAULT, WriteType.CACHE_THROUGH)
.setProperty(PropertyKey.UNDERFS_XATTR_CHANGE_ENABLED, false)
.setProperty(PropertyKey.USER_FILE_METADATA_SYNC_INTERVAL, 0)
.setProperty(PropertyKey.USER_FILE_METADATA_SYNC_INTERVAL, 0)
.setProperty(PropertyKey.DORA_WORKER_METASTORE_ROCKSDB_TTL, 0)
.setProperty(PropertyKey.DORA_UFS_LIST_STATUS_CACHE_TTL, 0)
.setProperty(PropertyKey.DORA_CLIENT_UFS_FALLBACK_ENABLED, false)
Expand Down
Loading