[
https://issues.apache.org/jira/browse/HDFS-16911?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17700859#comment-17700859
]
ASF GitHub Bot commented on HDFS-16911:
---------------------------------------
steveloughran commented on code in PR #5364:
URL: https://github.com/apache/hadoop/pull/5364#discussion_r1137750976
##########
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:
are you expecting an exception here? if so, intercept() has an overload
where you declare text to look for
##########
hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpConstants.java:
##########
@@ -122,8 +122,6 @@ private DistCpConstants() {
/* DistCp CopyListing class override param */
public static final String CONF_LABEL_COPY_LISTING_CLASS =
"distcp.copy.listing.class";
- public static final String CONF_LABEL_DISTCP_SYNC_CLASS =
"distcp.sync.class";
Review Comment:
reinstate. api is tagged as @InterfaceAudience.LimitedPrivate("Distcp
support tools"). if it's not supported tag as deprecated, but ideally have it
keep working
##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshotDiffReport.java:
##########
@@ -253,6 +253,11 @@ public String getFromSnapshot() {
return fromSnapshot;
}
+ /** @return {@link #toSnapshot} */
Review Comment:
nit add a .
##########
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:
tell us the filesystem
> 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: [email protected]
For additional commands, e-mail: [email protected]
