[ 
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

Reply via email to