From eb54f19a2b65c3e6aa85b9882039cd7019b57e4b Mon Sep 17 00:00:00 2001 From: Abhishek Das Date: Tue, 9 Nov 2021 01:03:11 -0800 Subject: [PATCH 1/2] HADOOP-17999: Do not initialize all target FileSystems for setWriteChecksum and setVerifyChecksum --- .../hadoop/fs/viewfs/ViewFileSystem.java | 34 +++++++++------ .../fs/viewfs/ViewFileSystemBaseTest.java | 43 +++++++++++++++++++ 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index ce918a13ea2ac..80a4174e9df41 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -239,6 +239,8 @@ public URI[] getTargetFileSystemURIs() { Path homeDir = null; private boolean enableInnerCache = false; private InnerCache cache; + private Boolean setVerifyChecksum = null; + private Boolean setWriteChecksum = null; // Default to rename within same mountpoint private RenameStrategy renameStrategy = RenameStrategy.SAME_MOUNTPOINT; /** @@ -338,6 +340,12 @@ public FileSystem run() throws IOException { } } }); + if (setVerifyChecksum != null) { + fs.setVerifyChecksum(setVerifyChecksum); + } + if (setWriteChecksum != null) { + fs.setWriteChecksum(setWriteChecksum); + } return new ChRootedFileSystem(fs, uri); } catch (IOException | InterruptedException ex) { LOG.error("Could not initialize the underlying FileSystem " @@ -917,12 +925,13 @@ public void removeXAttr(Path path, String name) throws IOException { } @Override - public void setVerifyChecksum(final boolean verifyChecksum) { - List> mountPoints = - fsState.getMountPoints(); - Map fsMap = initializeMountedFileSystems(mountPoints); - for (InodeTree.MountPoint mount : mountPoints) { - fsMap.get(mount.src).setVerifyChecksum(verifyChecksum); + public void setVerifyChecksum(final boolean verifyChecksum) { + //Set the value for filesystems to be lazily initialized later l + setVerifyChecksum = verifyChecksum; + // Set verifyChecksum for filesystems already initialized before + // This will only work if the inner cache is enabled + for (FileSystem childFileSystems : cache.map.values()) { + childFileSystems.setVerifyChecksum(verifyChecksum); } } @@ -1019,12 +1028,13 @@ public QuotaUsage getQuotaUsage(Path f) throws IOException { } @Override - public void setWriteChecksum(final boolean writeChecksum) { - List> mountPoints = - fsState.getMountPoints(); - Map fsMap = initializeMountedFileSystems(mountPoints); - for (InodeTree.MountPoint mount : mountPoints) { - fsMap.get(mount.src).setWriteChecksum(writeChecksum); + public void setWriteChecksum(final boolean writeChecksum) { + //Set the value for filesystems to be lazily initialized later + setWriteChecksum = writeChecksum; + // Set verifyChecksum for filesystems already initialized before + // This will only work if the inner cache is enabled + for (FileSystem childFileSystem : cache.map.values()) { + childFileSystem.setWriteChecksum(writeChecksum); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java index be50f457be21c..434eff0bef32e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java @@ -1472,4 +1472,47 @@ public void testTargetFileSystemLazyInitialization() throws Exception { // viewfs inner cache is disabled assertEquals(cacheSize + 2, TestFileUtil.getCacheSize()); } + + @Test + public void testTargetFileSystemLazyInitializationForChecksumMethods() + throws Exception { + final String clusterName = "cluster" + new Random().nextInt(); + Configuration config = new Configuration(conf); + config.setBoolean(CONFIG_VIEWFS_ENABLE_INNER_CACHE, false); + config.setClass("fs.othermockfs.impl", + TestChRootedFileSystem.MockFileSystem.class, FileSystem.class); + ConfigUtil.addLink(config, clusterName, "/user", + URI.create("othermockfs://mockauth1/mockpath")); + ConfigUtil.addLink(config, clusterName, + "/mock", URI.create("othermockfs://mockauth/mockpath")); + + final int cacheSize = TestFileUtil.getCacheSize(); + ViewFileSystem viewFs = (ViewFileSystem) FileSystem.get( + new URI("viewfs://" + clusterName + "/"), config); + + // As no inner file system instance has been initialized, + // cache size will remain the same + // cache is disabled for viewfs scheme, so the viewfs:// instance won't + // go in the cache even after the initialization + assertEquals(cacheSize, TestFileUtil.getCacheSize()); + + // This is not going to initialize any filesystem instance + viewFs.setVerifyChecksum(true); + + // Cache size will remain the same + assertEquals(cacheSize, TestFileUtil.getCacheSize()); + + // This resolve path will initialize the file system corresponding + // to the mount table entry of the path "/user" + viewFs.getFileChecksum( + new Path(String.format("viewfs://%s/%s", clusterName, "/user"))); + + // Cache size will increase by 1. + assertEquals(cacheSize + 1, TestFileUtil.getCacheSize()); + + viewFs.close(); + // Initialized FileSystem instances will not be removed from cache as + // viewfs inner cache is disabled + assertEquals(cacheSize + 1, TestFileUtil.getCacheSize()); + } } From e6621db13de437453dffc921413f679613bd8a7e Mon Sep 17 00:00:00 2001 From: Abhishek Das Date: Mon, 15 Nov 2021 10:38:40 -0800 Subject: [PATCH 2/2] HADOOP-17999: No-op implementation of setWriteChecksum and setVerifyChecksum in ViewFileSystem The same has already been done in ViewFs. --- .../hadoop/fs/viewfs/ViewFileSystem.java | 26 +++---------------- .../viewfs/TestViewFileSystemDelegation.java | 12 --------- 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java index 80a4174e9df41..538f03a1300a3 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java @@ -239,8 +239,6 @@ public URI[] getTargetFileSystemURIs() { Path homeDir = null; private boolean enableInnerCache = false; private InnerCache cache; - private Boolean setVerifyChecksum = null; - private Boolean setWriteChecksum = null; // Default to rename within same mountpoint private RenameStrategy renameStrategy = RenameStrategy.SAME_MOUNTPOINT; /** @@ -340,12 +338,6 @@ public FileSystem run() throws IOException { } } }); - if (setVerifyChecksum != null) { - fs.setVerifyChecksum(setVerifyChecksum); - } - if (setWriteChecksum != null) { - fs.setWriteChecksum(setWriteChecksum); - } return new ChRootedFileSystem(fs, uri); } catch (IOException | InterruptedException ex) { LOG.error("Could not initialize the underlying FileSystem " @@ -926,13 +918,8 @@ public void removeXAttr(Path path, String name) throws IOException { @Override public void setVerifyChecksum(final boolean verifyChecksum) { - //Set the value for filesystems to be lazily initialized later l - setVerifyChecksum = verifyChecksum; - // Set verifyChecksum for filesystems already initialized before - // This will only work if the inner cache is enabled - for (FileSystem childFileSystems : cache.map.values()) { - childFileSystems.setVerifyChecksum(verifyChecksum); - } + // This is a file system level operations, however ViewFileSystem + // points to many file systems. Noop for ViewFileSystem. } /** @@ -1029,13 +1016,8 @@ public QuotaUsage getQuotaUsage(Path f) throws IOException { @Override public void setWriteChecksum(final boolean writeChecksum) { - //Set the value for filesystems to be lazily initialized later - setWriteChecksum = writeChecksum; - // Set verifyChecksum for filesystems already initialized before - // This will only work if the inner cache is enabled - for (FileSystem childFileSystem : cache.map.values()) { - childFileSystem.setWriteChecksum(writeChecksum); - } + // This is a file system level operations, however ViewFileSystem + // points to many file systems. Noop for ViewFileSystem. } @Override diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemDelegation.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemDelegation.java index d8c39f79d0454..3a60d6ecdda94 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemDelegation.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemDelegation.java @@ -83,12 +83,6 @@ public void testSanity() throws URISyntaxException { assertEquals(new URI("fs2:/").getAuthority(), fs2.getUri().getAuthority()); } - @Test - public void testVerifyChecksum() throws Exception { - checkVerifyChecksum(false); - checkVerifyChecksum(true); - } - /** * Tests that ViewFileSystem dispatches calls for every ACL method through the * mount table to the correct underlying FileSystem with all Path arguments @@ -144,12 +138,6 @@ public void testAclMethods() throws Exception { verify(mockFs2).getAclStatus(mockFsPath2); } - void checkVerifyChecksum(boolean flag) { - viewFs.setVerifyChecksum(flag); - assertEquals(flag, fs1.getVerifyChecksum()); - assertEquals(flag, fs2.getVerifyChecksum()); - } - static class FakeFileSystem extends LocalFileSystem { boolean verifyChecksum = true; URI uri;