HeartSaVioR commented on a change in pull request #1388: HADOOP-16255. Add
ChecksumFs.rename(path, path, boolean) to rename crc file as well when
FileContext.rename(path, path, options) is called.
URL: https://github.com/apache/hadoop/pull/1388#discussion_r319779730
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFs.java
##########
@@ -457,17 +458,35 @@ public boolean setReplication(Path src, short
replication)
@Override
public void renameInternal(Path src, Path dst)
throws IOException, UnresolvedLinkException {
+ renameInternal(src, dst, (s, d) -> getMyFs().rename(s, d));
+ }
+
+ @Override
+ public void renameInternal(Path src, Path dst, boolean overwrite)
+ throws AccessControlException, FileAlreadyExistsException,
+ FileNotFoundException, ParentNotDirectoryException,
+ UnresolvedLinkException, IOException {
+ Options.Rename renameOpt = Options.Rename.NONE;
+ if (overwrite) {
+ renameOpt = Options.Rename.OVERWRITE;
+ }
+ final Options.Rename opt = renameOpt;
+ renameInternal(src, dst, (s, d) -> getMyFs().rename(s, d, opt));
Review comment:
ChecksumFs should deal with checksum file by itself: that's why the method
should be override. Just delegating rename would have same result, checksum
file leak.
> What is the concrete class of the AbastractFileSystem field?
If the source file is from local filesystem, I think it's RawLocalFs. It
could be DelegateToFileSystem, but it will redirect to RawLocalFs eventually.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
