ayushtkn commented on pull request #2852:
URL: https://github.com/apache/hadoop/pull/2852#issuecomment-812918934
Apart from the checkstyle issue, changes LGTM.
Removal of code shouldn't bother, it is used in ``validatePaths`` which
won't break by passing the actual dest.
In the present code we were passing ``/NONE`` as target which actually
didn't exist considering we would be least bothered with the target path, when
building listing, but the problem seems to happen at
```SimpleCopyListing#computeSourceRootPath``` when the source is a ``solitary
file``, It has check which relies on target:
```
if (solitaryFile) {
if (!targetPathExists || targetFS.isFile(target)) {
return sourceStatus.getPath();
} else {
return sourceStatus.getPath().getParent();
}
```
So, here the ``targetFS.isFile(target)`` returns `false` due to ``FNF`` on
``/NONE`` which wasn't in the case when building listing for ``source``, so for
source the listing is built with ``sourceStatus.getPath();`` and for target
with ``sourceStatus.getPath().getParent()``, hence on comparison in
``CopyCommitter`` file gets deleted.
So, this passing `/NONE` wasn't a good idea, The present fix to pass the
actual destination seems fair enough. Just give a check, guess these constants
can be removed:
These constants can be removed now?
```
/**
* Constants for NONE file deletion
*/
public static final String NONE_PATH_NAME = "/NONE";
public static final Path NONE_PATH = new Path(NONE_PATH_NAME);
public static final Path RAW_NONE_PATH = new Path(
DistCpConstants.HDFS_RESERVED_RAW_DIRECTORY_NAME + NONE_PATH_NAME);
```
Nice Catch!!!
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
