From ce588235b668635f9a154660b6eb91361dbcc65c Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Sun, 16 Jul 2023 23:05:58 +0800 Subject: [PATCH 1/4] HDFS-17089. Close child file systems in ViewFileSystem when cache is disabled. --- .../apache/hadoop/fs/viewfs/InodeTree.java | 5 ++ .../hadoop/fs/viewfs/ViewFileSystem.java | 29 ++++++++++++ .../fs/viewfs/TestViewFileSystemClose.java | 47 +++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java index 5360d55e106448..466057feec7182 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/InodeTree.java @@ -413,6 +413,11 @@ public T getTargetFileSystem() throws IOException { } return targetFileSystem; } + + T getTargetFileSystemForClose() throws IOException { + return targetFileSystem; + } + } private void createLink(final String src, final String target, 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 e31a701a6eaa71..486ece72f5c889 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 @@ -1926,6 +1926,21 @@ enum RenameStrategy { SAME_FILESYSTEM_ACROSS_MOUNTPOINT } + private void closeChildFileSystems(FileSystem fs) throws IOException { + if (fs != null) { + FileSystem[] childFs = fs.getChildFileSystems(); + for (FileSystem child : childFs) { + if (child != null) { + String disableCacheName = String.format("fs.%s.impl.disable.cache", + child.getUri().getScheme()); + if (config.getBoolean(disableCacheName, false)) { + child.close(); + } + } + } + } + } + @Override public void close() throws IOException { super.close(); @@ -1933,5 +1948,19 @@ public void close() throws IOException { cache.closeAll(); cache.clear(); } + + if (!enableInnerCache) { + for (InodeTree.MountPoint mountPoint : + fsState.getMountPoints()) { + FileSystem targetFs = mountPoint.target.getTargetFileSystemForClose(); + closeChildFileSystems(targetFs); + } + + if (fsState.isRootInternalDir() && + fsState.getRootFallbackLink() != null) { + closeChildFileSystems( + fsState.getRootFallbackLink().getTargetFileSystem()); + } + } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java new file mode 100644 index 00000000000000..247d76f558ad34 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java @@ -0,0 +1,47 @@ +package org.apache.hadoop.fs.viewfs; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.FsConstants; +import org.apache.hadoop.fs.Path; +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class TestViewFileSystemClose { + + /** + * Verify that all child file systems of a ViewFileSystem will be shut down + * when the cache is disabled. + * @throws IOException + */ + @Test + public void testFileSystemLeak() throws IOException { + + Configuration conf = new Configuration(); + conf.set("fs.viewfs.impl", ViewFileSystem.class.getName()); + conf.setBoolean("fs.viewfs.enable.inner.cache", false); + conf.setBoolean("fs.viewfs.impl.disable.cache", true); + conf.setBoolean("fs.hdfs.impl.disable.cache", true); + + String rootPath = "hdfs://localhost/tmp"; + ConfigUtil.addLink(conf, "/data", new Path(rootPath, "data").toUri()); + ViewFileSystem viewFs = + (ViewFileSystem) FileSystem.get(FsConstants.VIEWFS_URI, conf); + + FileSystem[] children = viewFs.getChildFileSystems(); + viewFs.close(); + FileSystem.closeAll(); + for (FileSystem fs : children) { + try { + fs.create(new Path(rootPath, "neverSuccess")); + fail(); + } catch (IOException ioe) { + assertTrue(ioe.getMessage().contains("Filesystem closed")); + } + } + } +} From e089effc204feea8b75479faae6a534261845efe Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Mon, 17 Jul 2023 10:15:09 +0800 Subject: [PATCH 2/4] Add license. --- .../fs/viewfs/TestViewFileSystemClose.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java index 247d76f558ad34..a66ec11a29970f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java @@ -1,3 +1,20 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.apache.hadoop.fs.viewfs; import org.apache.hadoop.conf.Configuration; From 6515ee70574b1d5e16266fe3ccb49356add68728 Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Tue, 18 Jul 2023 10:16:39 +0800 Subject: [PATCH 3/4] feedback review. --- .../fs/viewfs/TestViewFileSystemClose.java | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java index a66ec11a29970f..989787629cce9d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java @@ -17,18 +17,18 @@ */ package org.apache.hadoop.fs.viewfs; +import static org.apache.hadoop.test.LambdaTestUtils.intercept; + +import java.io.IOException; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FsConstants; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.test.AbstractHadoopTestBase; import org.junit.Test; -import java.io.IOException; - -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -public class TestViewFileSystemClose { +public class TestViewFileSystemClose extends AbstractHadoopTestBase { /** * Verify that all child file systems of a ViewFileSystem will be shut down @@ -36,7 +36,7 @@ public class TestViewFileSystemClose { * @throws IOException */ @Test - public void testFileSystemLeak() throws IOException { + public void testFileSystemLeak() throws Exception { Configuration conf = new Configuration(); conf.set("fs.viewfs.impl", ViewFileSystem.class.getName()); @@ -53,12 +53,9 @@ public void testFileSystemLeak() throws IOException { viewFs.close(); FileSystem.closeAll(); for (FileSystem fs : children) { - try { - fs.create(new Path(rootPath, "neverSuccess")); - fail(); - } catch (IOException ioe) { - assertTrue(ioe.getMessage().contains("Filesystem closed")); - } + intercept(IOException.class, "Filesystem closed", + "Expect Filesystem closed IOException", + () -> fs.create(new Path(rootPath, "neverSuccess"))); } } } From 73db01bbf8cfe1001c8a24fce4874ac4d71307ad Mon Sep 17 00:00:00 2001 From: zhangshuyan Date: Wed, 19 Jul 2023 18:30:53 +0800 Subject: [PATCH 4/4] feedback review. --- .../java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java | 9 +++++++-- .../apache/hadoop/fs/viewfs/TestViewFileSystemClose.java | 4 ++-- 2 files changed, 9 insertions(+), 4 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 486ece72f5c889..b4cf96ea5996a7 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 @@ -1926,7 +1926,7 @@ enum RenameStrategy { SAME_FILESYSTEM_ACROSS_MOUNTPOINT } - private void closeChildFileSystems(FileSystem fs) throws IOException { + private void closeChildFileSystems(FileSystem fs) { if (fs != null) { FileSystem[] childFs = fs.getChildFileSystems(); for (FileSystem child : childFs) { @@ -1934,7 +1934,12 @@ private void closeChildFileSystems(FileSystem fs) throws IOException { String disableCacheName = String.format("fs.%s.impl.disable.cache", child.getUri().getScheme()); if (config.getBoolean(disableCacheName, false)) { - child.close(); + try { + child.close(); + } catch (IOException e) { + LOG.info("Fail closing ViewFileSystem's child filesystem " + fs, + e); + } } } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java index 989787629cce9d..7301e0c0acfa3a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemClose.java @@ -17,8 +17,6 @@ */ package org.apache.hadoop.fs.viewfs; -import static org.apache.hadoop.test.LambdaTestUtils.intercept; - import java.io.IOException; import org.apache.hadoop.conf.Configuration; @@ -28,6 +26,8 @@ import org.apache.hadoop.test.AbstractHadoopTestBase; import org.junit.Test; +import static org.apache.hadoop.test.LambdaTestUtils.intercept; + public class TestViewFileSystemClose extends AbstractHadoopTestBase { /**