[ https://issues.apache.org/jira/browse/HDFS-16911?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17701244#comment-17701244 ]
ASF GitHub Bot commented on HDFS-16911: --------------------------------------- sadanand48 commented on code in PR #5364: URL: https://github.com/apache/hadoop/pull/5364#discussion_r1138906146 ########## hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpSync.java: ########## @@ -1276,4 +1283,63 @@ private void snapshotDiffWithPaths(Path sourceFSPath, verifyCopyByFs(sourceFS, targetFS, sourceFS.getFileStatus(sourceFSPath), targetFS.getFileStatus(targetFSPath), false); } + + @Test + public void testSyncSnapshotDiffWithLocalFileSystem() throws Exception { + String[] args = new String[]{"-update", "-diff", "s1", "s2", + "file:///source", "file:///target"}; + LambdaTestUtils.intercept( + IllegalArgumentException.class, + "The source file system file does not support snapshot", + () -> new DistCp(conf, OptionsParser.parse(args)).execute()); + } + + @Test + public void testSyncSnapshotDiffWithDummyFileSystem() throws Exception { + String[] args = + new String[] { "-update", "-diff", "s1", "s2", "dummy:///source", + "dummy:///target" }; + try { + FileSystem dummyFs = FileSystem.get(URI.create("dummy:///"), conf); + Assert.assertTrue(dummyFs instanceof DummyFs); + new DistCp(conf, OptionsParser.parse(args)).execute(); + } catch (Exception e) { + // can expect other exceptions as source and target paths + // are not created, assert if the exception is not arising + // due to the filesystem not supporting snapshots. + Assert.assertFalse(e.getMessage().contains("does not support snapshot")); Review Comment: Here I'm checking that a particular exception i.e exception stating that 'fs doesn't support snapshots' is **not** thrown, The intercept utility method checks whether the caught exception has a particular text, I think there is no util method to check its negation. ########## hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpSync.java: ########## @@ -169,20 +153,27 @@ protected boolean preSyncCheck() throws IOException { return true; } - protected void checkFilesystemSupport(FileSystem srcFs, FileSystem tgtFs) { - // currently we require both the source and the target file system are - // DistributedFileSystem or (S)WebHdfsFileSystem. - if (!(srcFs instanceof DistributedFileSystem - || srcFs instanceof WebHdfsFileSystem)) { - throw new IllegalArgumentException("Unsupported source file system: " - + srcFs.getScheme() + "://. " + - "Supported file systems: hdfs://, webhdfs:// and swebhdfs://."); + /** + * Check if the source and target filesystems support snapshots. + */ + protected void checkFilesystemSupport(Path sourceDir, Path targetDir, + FileSystem srcFs, FileSystem tgtFs) throws IOException { + if (!srcFs.hasPathCapability(sourceDir, + CommonPathCapabilities.FS_SNAPSHOTS)) { + throw new IllegalArgumentException( + "The source file system " + srcFs.getScheme() + " does not support snapshot."); + } + if (!tgtFs.hasPathCapability(targetDir, + CommonPathCapabilities.FS_SNAPSHOTS)) { + throw new IllegalArgumentException( + "The target file system " + tgtFs.getScheme() + " does not support snapshot."); } - if (!(tgtFs instanceof DistributedFileSystem - || tgtFs instanceof WebHdfsFileSystem)) { - throw new IllegalArgumentException("Unsupported target file system: " - + tgtFs.getScheme() + "://. " + - "Supported file systems: hdfs://, webhdfs:// and swebhdfs://."); + try { + getSnapshotDiffReportMethod(srcFs); + getSnapshotDiffReportMethod(tgtFs); + } catch (NoSuchMethodException e) { + throw new IllegalArgumentException( Review Comment: Done. > Distcp with snapshot diff to support Ozone filesystem. > ------------------------------------------------------ > > Key: HDFS-16911 > URL: https://issues.apache.org/jira/browse/HDFS-16911 > Project: Hadoop HDFS > Issue Type: Bug > Components: distcp > Reporter: Sadanand Shenoy > Priority: Major > Labels: pull-request-available > > Currently in DistcpSync i.e the step which applies the diff b/w 2 provided > snapshots as arguments to the distcp job with -diff option, only > DistributedFilesystem and WebHDFS filesytems are supported. > > {code:java} > // currently we require both the source and the target file system are > // DistributedFileSystem or (S)WebHdfsFileSystem. > if (!(srcFs instanceof DistributedFileSystem > || srcFs instanceof WebHdfsFileSystem)) { > throw new IllegalArgumentException("Unsupported source file system: " > + srcFs.getScheme() + "://. " + > "Supported file systems: hdfs://, webhdfs:// and swebhdfs://."); > } > if (!(tgtFs instanceof DistributedFileSystem > || tgtFs instanceof WebHdfsFileSystem)) { > throw new IllegalArgumentException("Unsupported target file system: " > + tgtFs.getScheme() + "://. " + > "Supported file systems: hdfs://, webhdfs:// and swebhdfs://."); > }{code} > As Ozone now supports snapshot feature after HDDS-6517, add support to use > distcp with ozone snapshots. > -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org