This is an automated email from the ASF dual-hosted git repository. ckj pushed a commit to branch ozone-1.3 in repository https://gitbox.apache.org/repos/asf/ozone.git
commit d9f5b29850288966b9fa311a6503b6d8030ed4d3 Author: Doroszlai, Attila <[email protected]> AuthorDate: Wed Feb 22 10:53:56 2023 +0100 HDDS-7991. Do not return fake parent dir for deleted keys (#4287) --- .../ozone/contract/ITestOzoneContractCreate.java | 23 ++++---- .../ozone/contract/ITestOzoneContractDelete.java | 23 ++++---- .../contract/ITestOzoneContractGetFileStatus.java | 28 +++++----- .../fs/ozone/contract/ITestOzoneContractMkdir.java | 23 ++++---- .../fs/ozone/contract/ITestOzoneContractOpen.java | 23 ++++---- .../ozone/contract/ITestOzoneContractRename.java | 23 ++++---- .../ozone/contract/ITestOzoneContractRootDir.java | 23 ++++---- .../fs/ozone/contract/ITestOzoneContractUtils.java | 61 ---------------------- .../hadoop/fs/ozone/contract/OzoneContract.java | 39 +++++++++----- .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 49 +++++++++++++---- 10 files changed, 144 insertions(+), 171 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractCreate.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractCreate.java index 034cf1e185..fd4e4d416f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractCreate.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractCreate.java @@ -35,19 +35,18 @@ import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ITestOzoneContractCreate extends AbstractContractCreateTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractCreate(boolean fso) { + // Actual init done in initParam(). + } - public ITestOzoneContractCreate(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); } - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractCreate.fsOptimizedServer = fsOptimizedServer; + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } @AfterClass @@ -61,7 +60,7 @@ public class ITestOzoneContractCreate extends AbstractContractCreateTest { } @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractDelete.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractDelete.java index 1381a2cda8..8ca70f0a7f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractDelete.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractDelete.java @@ -35,19 +35,18 @@ import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ITestOzoneContractDelete extends AbstractContractDeleteTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractDelete(boolean fso) { + // Actual init done in initParam(). + } - public ITestOzoneContractDelete(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); } - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractDelete.fsOptimizedServer = fsOptimizedServer; + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } @AfterClass @@ -61,7 +60,7 @@ public class ITestOzoneContractDelete extends AbstractContractDeleteTest { } @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractGetFileStatus.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractGetFileStatus.java index 04a3fb5a06..a8013387cd 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractGetFileStatus.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractGetFileStatus.java @@ -38,25 +38,23 @@ import org.slf4j.LoggerFactory; public class ITestOzoneContractGetFileStatus extends AbstractContractGetFileStatusTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractGetFileStatus(boolean fso) { + // Actual init done in initParam(). + } + + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); + } - public ITestOzoneContractGetFileStatus(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } private static final Logger LOG = LoggerFactory.getLogger(ITestOzoneContractGetFileStatus.class); - - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractGetFileStatus.fsOptimizedServer = fsOptimizedServer; - } - @AfterClass public static void teardownCluster() throws IOException { OzoneContract.destroyCluster(); @@ -79,7 +77,7 @@ public class ITestOzoneContractGetFileStatus } @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractMkdir.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractMkdir.java index 862b2b9ede..49118a0595 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractMkdir.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractMkdir.java @@ -35,19 +35,18 @@ import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ITestOzoneContractMkdir extends AbstractContractMkdirTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractMkdir(boolean fso) { + // Actual init done in initParam(). + } - public ITestOzoneContractMkdir(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); } - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractMkdir.fsOptimizedServer = fsOptimizedServer; + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } @AfterClass @@ -61,7 +60,7 @@ public class ITestOzoneContractMkdir extends AbstractContractMkdirTest { } @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractOpen.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractOpen.java index 83a6306fa4..05babc015a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractOpen.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractOpen.java @@ -35,19 +35,18 @@ import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ITestOzoneContractOpen extends AbstractContractOpenTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractOpen(boolean fso) { + // Actual init done in initParam(). + } - public ITestOzoneContractOpen(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); } - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractOpen.fsOptimizedServer = fsOptimizedServer; + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } @AfterClass @@ -61,7 +60,7 @@ public class ITestOzoneContractOpen extends AbstractContractOpenTest { } @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRename.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRename.java index 2fa1c64965..fe5c112a10 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRename.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRename.java @@ -35,19 +35,18 @@ import org.junit.runners.Parameterized; @RunWith(Parameterized.class) public class ITestOzoneContractRename extends AbstractContractRenameTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractRename(boolean fso) { + // Actual init done in initParam(). + } - public ITestOzoneContractRename(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); } - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractRename.fsOptimizedServer = fsOptimizedServer; + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } @AfterClass @@ -62,7 +61,7 @@ public class ITestOzoneContractRename extends AbstractContractRenameTest { @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRootDir.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRootDir.java index 5ca5bc3c42..f4ec389229 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRootDir.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractRootDir.java @@ -36,19 +36,18 @@ import org.junit.runners.Parameterized; public class ITestOzoneContractRootDir extends AbstractContractRootDirectoryTest { - private static boolean fsOptimizedServer; + public ITestOzoneContractRootDir(boolean fso) { + // Actual init done in initParam(). + } - public ITestOzoneContractRootDir(boolean fsoServer) - throws IOException { - if (fsOptimizedServer != fsoServer) { - setFsOptimizedServer(fsoServer); - ITestOzoneContractUtils.restartCluster( - fsOptimizedServer); - } + @Parameterized.BeforeParam + public static void initParam(boolean fso) throws IOException { + OzoneContract.createCluster(fso); } - public static void setFsOptimizedServer(boolean fsOptimizedServer) { - ITestOzoneContractRootDir.fsOptimizedServer = fsOptimizedServer; + @Parameterized.AfterParam + public static void teardownParam() throws IOException { + OzoneContract.destroyCluster(); } @AfterClass @@ -62,8 +61,8 @@ public class ITestOzoneContractRootDir extends } @Parameterized.Parameters - public static Collection data() { - return ITestOzoneContractUtils.getFsoCombinations(); + public static Collection<Boolean> data() { + return OzoneContract.getFsoCombinations(); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractUtils.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractUtils.java deleted file mode 100644 index f7858d1e2a..0000000000 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/ITestOzoneContractUtils.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * 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.ozone.contract; - - -import java.io.IOException; -import java.util.Arrays; -import java.util.List; - -/** - * Utility class for Ozone-contract tests. - */ -public final class ITestOzoneContractUtils { - - private ITestOzoneContractUtils() { } - - private static List<Object> fsoCombinations = Arrays.asList(new Object[] { - // FSO configuration is a cluster level server side configuration. - // If the cluster is configured with SIMPLE metadata layout, - // non-FSO bucket will created. - // If the cluster is configured with PREFIX metadata layout, - // FSO bucket will be created. - // Presently, OzoneClient checks bucketMetadata then invokes FSO or - // non-FSO specific code and it makes no sense to add client side - // configs now. Once the specific client API to set FSO or non-FSO - // bucket is provided the contract test can be refactored to include - // another parameter (fsoClient) which sets/unsets the client side - // configs. - true, // Server is configured with new layout (PREFIX) - // and new buckets will be operated on - false // Server is configured with old layout (SIMPLE) - // and old buckets will be operated on - }); - - static List<Object> getFsoCombinations() { - return fsoCombinations; - } - - public static void restartCluster(boolean fsOptimizedServer) - throws IOException { - OzoneContract.destroyCluster(); - OzoneContract.initOzoneConfiguration( - fsOptimizedServer); - OzoneContract.createCluster(); - } -} diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/OzoneContract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/OzoneContract.java index 784897aae7..c68258972f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/OzoneContract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/contract/OzoneContract.java @@ -20,6 +20,8 @@ package org.apache.hadoop.fs.ozone.contract; import java.io.IOException; import java.time.Duration; +import java.util.Arrays; +import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -44,6 +46,23 @@ import org.junit.Assert; */ class OzoneContract extends AbstractFSContract { + private static final List<Boolean> FSO_COMBINATIONS = Arrays.asList( + // FSO configuration is a cluster level server side configuration. + // If the cluster is configured with SIMPLE metadata layout, + // non-FSO bucket will created. + // If the cluster is configured with PREFIX metadata layout, + // FSO bucket will be created. + // Presently, OzoneClient checks bucketMetadata then invokes FSO or + // non-FSO specific code and it makes no sense to add client side + // configs now. Once the specific client API to set FSO or non-FSO + // bucket is provided the contract test can be refactored to include + // another parameter (fsoClient) which sets/unsets the client side + // configs. + true, // Server is configured with new layout (PREFIX) + // and new buckets will be operated on + false // Server is configured with old layout (SIMPLE) + // and old buckets will be operated on + ); private static MiniOzoneCluster cluster; private static final String CONTRACT_XML = "contract/ozone.xml"; @@ -55,6 +74,10 @@ class OzoneContract extends AbstractFSContract { addConfResource(CONTRACT_XML); } + static List<Boolean> getFsoCombinations() { + return FSO_COMBINATIONS; + } + @Override public String getScheme() { return OzoneConsts.OZONE_URI_SCHEME; @@ -62,8 +85,7 @@ class OzoneContract extends AbstractFSContract { @Override public Path getTestPath() { - Path path = new Path("/test"); - return path; + return new Path("/test"); } public static void initOzoneConfiguration(boolean fsoServer) { @@ -92,16 +114,9 @@ class OzoneContract extends AbstractFSContract { conf.addResource(CONTRACT_XML); - if (fsOptimizedServer) { - // Default bucket layout is set to FSO in case of FSO server. - conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT, - OMConfigKeys.OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED); - } else { - // Default bucket layout is set to LEGACY to support Hadoop compatible - // FS operations that are incompatible with OBS (default config value). - conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT, - BucketLayout.LEGACY.name()); - } + BucketLayout bucketLayout = fsOptimizedServer + ? BucketLayout.FILE_SYSTEM_OPTIMIZED : BucketLayout.LEGACY; + conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT, bucketLayout.name()); cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(5).build(); try { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index 51663a0ac1..bd87bf0084 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -1296,24 +1296,51 @@ public class KeyManagerImpl implements KeyManager { */ private OmKeyInfo createFakeDirIfShould(String volume, String bucket, String keyName, BucketLayout layout) throws IOException { - OmKeyInfo fakeDirKeyInfo = null; String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName); String targetKey = OzoneFSUtils.addTrailingSlashIfNeeded( metadataManager.getOzoneKey(volume, bucket, keyName)); + + Table<String, OmKeyInfo> keyTable = metadataManager.getKeyTable(layout); + Iterator<Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>>> cacheIterator = + keyTable.cacheIterator(); + while (cacheIterator.hasNext()) { + Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>> cacheEntry = + cacheIterator.next(); + String cacheKey = cacheEntry.getKey().getCacheKey(); + CacheValue<OmKeyInfo> cacheValue = cacheEntry.getValue(); + boolean exists = cacheValue != null && cacheValue.getCacheValue() != null; + if (exists + && cacheKey.startsWith(targetKey) + && !Objects.equals(cacheKey, targetKey)) { + LOG.debug("Fake dir {} required for {}", targetKey, cacheKey); + return createDirectoryKey(cacheValue.getCacheValue(), dirKey); + } + } + try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> - keyTblItr = metadataManager.getKeyTable(layout).iterator()) { - Table.KeyValue<String, OmKeyInfo> keyValue = keyTblItr.seek(targetKey); - - // HDDS-7871: RocksIterator#seek() may position at the key - // past the target, we should check the full dbKeyName. - // For example, seeking "/vol1/bucket1/dir2/" may return a key - // in different volume/bucket, such as "/vol1/bucket2/dir2/key2". - if (keyValue != null && keyValue.getKey().startsWith(targetKey)) { - fakeDirKeyInfo = createDirectoryKey(keyValue.getValue(), dirKey); + keyTblItr = keyTable.iterator(targetKey)) { + while (keyTblItr.hasNext()) { + Table.KeyValue<String, OmKeyInfo> keyValue = keyTblItr.next(); + if (keyValue != null) { + String key = keyValue.getKey(); + // HDDS-7871: RocksIterator#seek() may position at the key + // past the target, we should check the full dbKeyName. + // For example, seeking "/vol1/bucket1/dir2/" may return a key + // in different volume/bucket, such as "/vol1/bucket2/dir2/key2". + if (key.startsWith(targetKey)) { + if (!Objects.equals(key, targetKey) + && !isKeyDeleted(key, keyTable)) { + LOG.debug("Fake dir {} required for {}", targetKey, key); + return createDirectoryKey(keyValue.getValue(), dirKey); + } + } else { + break; + } + } } } - return fakeDirKeyInfo; + return null; } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
