pkumarsinha commented on a change in pull request #2650:
URL: https://github.com/apache/hive/pull/2650#discussion_r715805381
##########
File path:
shims/0.23/src/main/test/org/apache/hadoop/hive/shims/TestHadoop23Shims.java
##########
@@ -94,6 +96,96 @@ public void testConstructDistCpParams() {
}
+ @Test
+ public void testXAttrNotPreservedDueToDestFS() throws Exception {
+ FileSystem fsWithXAttrSupport = mock(FileSystem.class);
+ FileSystem fsWithoutXAttrSupport = mock(FileSystem.class);
+ when(fsWithXAttrSupport.getXAttrs(any())).thenReturn(new HashMap<>());
+ when(fsWithoutXAttrSupport.getXAttrs(any())).thenThrow(
+ new UnsupportedOperationException("XAttr not supported for file
system."));
+ Configuration conf = new Configuration();
+ Path copySrc = mock(Path.class);
+ when(copySrc.getFileSystem(any())).thenReturn(fsWithXAttrSupport);
+ Path copyDst = mock(Path.class);
+ when(copyDst.getFileSystem(any())).thenReturn(fsWithoutXAttrSupport);
+
+ Hadoop23Shims shims = new Hadoop23Shims();
+ List<String> paramsDefault =
shims.constructDistCpParams(Collections.singletonList(copySrc), copyDst, conf);
+
+ assertEquals(5, paramsDefault.size());
+ assertTrue("Distcp -pb set by default", paramsDefault.contains("-pb"));
+ assertTrue("Distcp -update set by default",
paramsDefault.contains("-update"));
+ assertTrue("Distcp -delete set by default",
paramsDefault.contains("-delete"));
+ assertEquals(copySrc.toString(), paramsDefault.get(3));
+ assertEquals(copyDst.toString(), paramsDefault.get(4));
+ }
+
+ @Test
+ public void testXAttrNotPreservedDueToSrcFS() throws Exception {
+ FileSystem fsWithXAttrSupport = mock(FileSystem.class);
+ FileSystem fsWithoutXAttrSupport = mock(FileSystem.class);
+ when(fsWithXAttrSupport.getXAttrs(any())).thenReturn(new HashMap<>());
+ when(fsWithoutXAttrSupport.getXAttrs(any())).thenThrow(
+ new UnsupportedOperationException("XAttr not supported for file
system."));
+ Configuration conf = new Configuration();
+ Path copySrc = mock(Path.class);
+ when(copySrc.getFileSystem(any())).thenReturn(fsWithoutXAttrSupport);
+ Path copyDst = mock(Path.class);
+ when(copyDst.getFileSystem(any())).thenReturn(fsWithXAttrSupport);
Review comment:
Whole mocking can be moved to a method and reused. We should avoid
duplication of code.
##########
File path:
shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
##########
@@ -1192,12 +1204,15 @@ public Boolean run() throws Exception {
@Override
public boolean runDistCp(List<Path> srcPaths, Path dst, Configuration conf)
throws IOException {
- DistCpOptions options = new DistCpOptions.Builder(srcPaths, dst)
- .withSyncFolder(true)
- .withDeleteMissing(true)
- .preserve(FileAttribute.BLOCKSIZE)
- .preserve(FileAttribute.XATTR)
- .build();
+ DistCpOptions.Builder builder = new DistCpOptions.Builder(srcPaths, dst)
+ .withSyncFolder(true)
+ .withDeleteMissing(true)
+ .preserve(FileAttribute.BLOCKSIZE);
+ if (checkFileSystemXAttrSupport(dst.getFileSystem(conf))
+ &&
checkFileSystemXAttrSupport(srcPaths.get(0).getFileSystem(conf))) {
+ builder = builder.preserve(FileAttribute.XATTR);
+ }
+ DistCpOptions options = builder.build();
Review comment:
What is the difference between option and params returned by
constructDistCpParams? Why are we passing preserve attribute in both. Can one
not suffice?
##########
File path:
shims/0.23/src/main/test/org/apache/hadoop/hive/shims/TestHadoop23Shims.java
##########
@@ -94,6 +100,70 @@ public void testConstructDistCpParams() {
}
+ @Test
+ public void testXAttrNotPreservedDueToDestFS() throws Exception {
+ Configuration conf = new Configuration();
+ Path copySrc = mock(Path.class);
+ FileSystem srcFs = new
MiniDFSCluster.Builder(conf).numDataNodes(1).build().getFileSystem();
+ assertTrue("srcFs supports XAttrs.", srcFs.getXAttrs(new
Path(Path.SEPARATOR)) != null);
Review comment:
What type of FS is this here in case of MiniHdfs? Why is this important.
Also, could n't we have used same interface checkFileSystemXAttrSupport()?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
