You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ra...@apache.org on 2021/04/07 03:25:38 UTC

[ozone] 11/29: HDDS-4717. Fix TestOzoneFileSystemV1 and TestObjectStoreV1 cases (#1815)

This is an automated email from the ASF dual-hosted git repository.

rakeshr pushed a commit to branch HDDS-2939
in repository https://gitbox.apache.org/repos/asf/ozone.git

commit ea5b39679e57efdd6dcaf7a76e3a4bd035290c28
Author: Rakesh Radhakrishnan <ra...@apache.org>
AuthorDate: Fri Jan 22 21:15:11 2021 +0530

    HDDS-4717. Fix TestOzoneFileSystemV1 and TestObjectStoreV1 cases (#1815)
---
 .../hadoop/ozone/client/io/KeyOutputStream.java    |   8 +
 .../apache/hadoop/fs/ozone/TestOzoneDirectory.java |  26 ---
 .../hadoop/fs/ozone/TestOzoneFileSystem.java       |   9 +-
 .../apache/hadoop/ozone/om/TestObjectStoreV1.java  | 192 ++++++++++++---------
 .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 112 ++++++++----
 .../ozone/om/request/key/OMKeyDeleteRequestV1.java |   6 +
 6 files changed, 215 insertions(+), 138 deletions(-)

diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
index 96a4c42..ff7b7fd 100644
--- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
+++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java
@@ -92,6 +92,8 @@ public class KeyOutputStream extends OutputStream {
   private boolean isException;
   private final BlockOutputStreamEntryPool blockOutputStreamEntryPool;
 
+  private long clientID;
+
   /**
    * A constructor for testing purpose only.
    */
@@ -127,6 +129,11 @@ public class KeyOutputStream extends OutputStream {
     return retryCount;
   }
 
+  @VisibleForTesting
+  public long getClientID() {
+    return clientID;
+  }
+
   @SuppressWarnings({"parameternumber", "squid:S00107"})
   public KeyOutputStream(
       OzoneClientConfig config,
@@ -158,6 +165,7 @@ public class KeyOutputStream extends OutputStream {
     this.retryCount = 0;
     this.isException = false;
     this.writeOffset = 0;
+    this.clientID = handler.getId();
   }
 
   /**
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java
index 87e9f09..56c6177 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneDirectory.java
@@ -23,8 +23,6 @@ import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
-import org.apache.hadoop.hdds.utils.db.Table;
-import org.apache.hadoop.hdds.utils.db.TableIterator;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
 import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.TestDataUtil;
@@ -33,7 +31,6 @@ import org.apache.hadoop.ozone.om.OMConfigKeys;
 import org.apache.hadoop.ozone.om.OMMetadataManager;
 import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
 import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
-import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.util.StringUtils;
 import org.junit.After;
 import org.junit.Assert;
@@ -49,7 +46,6 @@ import java.util.concurrent.TimeoutException;
 
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
 import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE;
-import static org.junit.Assert.fail;
 
 /**
  * Test verifies the entries and operations in directory table.
@@ -95,28 +91,6 @@ public class TestOzoneDirectory {
     Assert.assertEquals("Wrong OM numKeys metrics",
             4, cluster.getOzoneManager().getMetrics().getNumKeys());
 
-    // verify entries in directory table
-    TableIterator<String, ? extends
-            Table.KeyValue<String, OmDirectoryInfo>> iterator =
-            omMgr.getDirectoryTable().iterator();
-    iterator.seekToFirst();
-    int count = dirKeys.size();
-    Assert.assertEquals("Unexpected directory table entries!", 4, count);
-    while (iterator.hasNext()) {
-      count--;
-      Table.KeyValue<String, OmDirectoryInfo> value = iterator.next();
-      verifyKeyFormat(value.getKey(), dirKeys);
-    }
-    Assert.assertEquals("Unexpected directory table entries!", 0, count);
-
-    // verify entries in key table
-    TableIterator<String, ? extends
-            Table.KeyValue<String, OmKeyInfo>> keyTableItr =
-            omMgr.getKeyTable().iterator();
-    while (keyTableItr.hasNext()) {
-      fail("Shouldn't add any entries in KeyTable!");
-    }
-
     // create sub-dirs under same parent
     Path subDir5 = new Path("/d1/d2/d3/d4/d5");
     fs.mkdirs(subDir5);
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
index c830e07..09118b9 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
@@ -599,10 +599,17 @@ public class TestOzoneFileSystem {
     ArrayList<String> actualPathList = new ArrayList<>();
     if (rootItemCount != fileStatuses.length) {
       for (int i = 0; i < fileStatuses.length; i++) {
-        actualPaths.add(fileStatuses[i].getPath().getName());
+        boolean duplicate =
+                actualPaths.add(fileStatuses[i].getPath().getName());
+        if (!duplicate) {
+          LOG.info("Duplicate path:{} in FileStatusList",
+                  fileStatuses[i].getPath().getName());
+        }
         actualPathList.add(fileStatuses[i].getPath().getName());
       }
       if (rootItemCount != actualPathList.size()) {
+        LOG.info("actualPathsSize: {}", actualPaths.size());
+        LOG.info("actualPathListSize: {}", actualPathList.size());
         actualPaths.removeAll(paths);
         actualPathList.removeAll(paths);
         LOG.info("actualPaths: {}", actualPaths);
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java
index ee127cf..b88bbc3 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreV1.java
@@ -17,17 +17,23 @@
 package org.apache.hadoop.ozone.om;
 
 import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdds.client.ReplicationFactor;
 import org.apache.hadoop.hdds.client.ReplicationType;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.utils.db.Table;
-import org.apache.hadoop.hdds.utils.db.TableIterator;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.OzoneConsts;
+import org.apache.hadoop.ozone.TestDataUtil;
 import org.apache.hadoop.ozone.client.ObjectStore;
 import org.apache.hadoop.ozone.client.OzoneBucket;
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.client.OzoneKeyDetails;
 import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.client.io.KeyOutputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
 import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
@@ -36,6 +42,7 @@ import org.apache.hadoop.ozone.om.request.TestOMRequestUtils;
 import org.apache.hadoop.util.StringUtils;
 import org.junit.Assert;
 import org.junit.AfterClass;
+import org.junit.After;
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
@@ -45,6 +52,8 @@ import java.io.IOException;
 import java.util.HashMap;
 import java.util.UUID;
 
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE;
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
@@ -55,6 +64,9 @@ public class TestObjectStoreV1 {
   private static String clusterId;
   private static String scmId;
   private static String omId;
+  private static String volumeName;
+  private static String bucketName;
+  private static FileSystem fs;
 
   @Rule
   public Timeout timeout = new Timeout(240000);
@@ -78,12 +90,51 @@ public class TestObjectStoreV1 {
             .setOmId(omId)
             .build();
     cluster.waitForClusterToBeReady();
+    // create a volume and a bucket to be used by OzoneFileSystem
+    OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(cluster);
+    volumeName = bucket.getVolumeName();
+    bucketName = bucket.getName();
+
+    String rootPath = String.format("%s://%s.%s/",
+            OzoneConsts.OZONE_URI_SCHEME, bucket.getName(),
+            bucket.getVolumeName());
+    // Set the fs.defaultFS and start the filesystem
+    conf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+    // Set the number of keys to be processed during batch operate.
+    conf.setInt(OZONE_FS_ITERATE_BATCH_SIZE, 5);
+    fs = FileSystem.get(conf);
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    deleteRootDir();
+  }
+
+  /**
+   * Cleanup files and directories.
+   *
+   * @throws IOException DB failure
+   */
+  private void deleteRootDir() throws IOException {
+    Path root = new Path("/");
+    FileStatus[] fileStatuses = fs.listStatus(root);
+
+    if (fileStatuses == null) {
+      return;
+    }
+
+    for (FileStatus fStatus : fileStatuses) {
+      fs.delete(fStatus.getPath(), true);
+    }
+
+    fileStatuses = fs.listStatus(root);
+    if (fileStatuses != null) {
+      Assert.assertEquals("Delete root failed!", 0, fileStatuses.length);
+    }
   }
 
   @Test
   public void testCreateKey() throws Exception {
-    String volumeName = "volume" + RandomStringUtils.randomNumeric(5);
-    String bucketName = "bucket" + RandomStringUtils.randomNumeric(5);
     String parent = "a/b/c/";
     String file = "key" + RandomStringUtils.randomNumeric(5);
     String key = parent + file;
@@ -91,74 +142,67 @@ public class TestObjectStoreV1 {
     OzoneClient client = cluster.getClient();
 
     ObjectStore objectStore = client.getObjectStore();
-    objectStore.createVolume(volumeName);
-
     OzoneVolume ozoneVolume = objectStore.getVolume(volumeName);
     Assert.assertTrue(ozoneVolume.getName().equals(volumeName));
-    ozoneVolume.createBucket(bucketName);
-
     OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName);
     Assert.assertTrue(ozoneBucket.getName().equals(bucketName));
 
-    Table<String, OmKeyInfo> openKeyTable =
+    Table<String, OmKeyInfo> openFileTable =
             cluster.getOzoneManager().getMetadataManager().getOpenKeyTable();
 
     // before file creation
-    verifyKeyInFileTable(openKeyTable, file, 0, true);
+    verifyKeyInFileTable(openFileTable, file, 0, true);
 
     String data = "random data";
     OzoneOutputStream ozoneOutputStream = ozoneBucket.createKey(key,
             data.length(), ReplicationType.RATIS, ReplicationFactor.ONE,
             new HashMap<>());
 
-    OmDirectoryInfo dirPathC = getDirInfo(volumeName, bucketName, parent);
+    KeyOutputStream keyOutputStream =
+            (KeyOutputStream) ozoneOutputStream.getOutputStream();
+    long clientID = keyOutputStream.getClientID();
+
+    OmDirectoryInfo dirPathC = getDirInfo(parent);
     Assert.assertNotNull("Failed to find dir path: a/b/c", dirPathC);
 
     // after file creation
-    verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(),
-            false);
+    verifyKeyInOpenFileTable(openFileTable, clientID, file,
+            dirPathC.getObjectID(), false);
 
     ozoneOutputStream.write(data.getBytes(), 0, data.length());
     ozoneOutputStream.close();
 
-    Table<String, OmKeyInfo> keyTable =
+    Table<String, OmKeyInfo> fileTable =
             cluster.getOzoneManager().getMetadataManager().getKeyTable();
-
     // After closing the file. File entry should be removed from openFileTable
     // and it should be added to fileTable.
-    verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), false);
-    verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(),
-            true);
+    verifyKeyInFileTable(fileTable, file, dirPathC.getObjectID(), false);
+    verifyKeyInOpenFileTable(openFileTable, clientID, file,
+            dirPathC.getObjectID(), true);
 
     ozoneBucket.deleteKey(key);
 
     // after key delete
-    verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), true);
-    verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(),
-            true);
+    verifyKeyInFileTable(fileTable, file, dirPathC.getObjectID(), true);
+    verifyKeyInOpenFileTable(openFileTable, clientID, file,
+            dirPathC.getObjectID(), true);
   }
 
   @Test
   public void testLookupKey() throws Exception {
-    String volumeName = "volume" + RandomStringUtils.randomNumeric(5);
-    String bucketName = "bucket" + RandomStringUtils.randomNumeric(5);
     String parent = "a/b/c/";
-    String file = "key" + RandomStringUtils.randomNumeric(5);
-    String key = parent + file;
+    String fileName = "key" + RandomStringUtils.randomNumeric(5);
+    String key = parent + fileName;
 
     OzoneClient client = cluster.getClient();
 
     ObjectStore objectStore = client.getObjectStore();
-    objectStore.createVolume(volumeName);
-
     OzoneVolume ozoneVolume = objectStore.getVolume(volumeName);
     Assert.assertTrue(ozoneVolume.getName().equals(volumeName));
-    ozoneVolume.createBucket(bucketName);
-
     OzoneBucket ozoneBucket = ozoneVolume.getBucket(bucketName);
     Assert.assertTrue(ozoneBucket.getName().equals(bucketName));
 
-    Table<String, OmKeyInfo> openKeyTable =
+    Table<String, OmKeyInfo> openFileTable =
             cluster.getOzoneManager().getMetadataManager().getOpenKeyTable();
 
     String data = "random data";
@@ -166,19 +210,23 @@ public class TestObjectStoreV1 {
             data.length(), ReplicationType.RATIS, ReplicationFactor.ONE,
             new HashMap<>());
 
-    OmDirectoryInfo dirPathC = getDirInfo(volumeName, bucketName, parent);
+    KeyOutputStream keyOutputStream =
+            (KeyOutputStream) ozoneOutputStream.getOutputStream();
+    long clientID = keyOutputStream.getClientID();
+
+    OmDirectoryInfo dirPathC = getDirInfo(parent);
     Assert.assertNotNull("Failed to find dir path: a/b/c", dirPathC);
 
     // after file creation
-    verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(),
-            false);
+    verifyKeyInOpenFileTable(openFileTable, clientID, fileName,
+            dirPathC.getObjectID(), false);
 
     ozoneOutputStream.write(data.getBytes(), 0, data.length());
 
     // open key
     try {
       ozoneBucket.getKey(key);
-      fail("Should throw exception as file is not visible and its still " +
+      fail("Should throw exception as fileName is not visible and its still " +
               "open for writing!");
     } catch (OMException ome) {
       // expected
@@ -190,34 +238,33 @@ public class TestObjectStoreV1 {
     OzoneKeyDetails keyDetails = ozoneBucket.getKey(key);
     Assert.assertEquals(key, keyDetails.getName());
 
-    Table<String, OmKeyInfo> keyTable =
+    Table<String, OmKeyInfo> fileTable =
             cluster.getOzoneManager().getMetadataManager().getKeyTable();
 
     // When closing the key, entry should be removed from openFileTable
     // and it should be added to fileTable.
-    verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), false);
-    verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(),
-            true);
+    verifyKeyInFileTable(fileTable, fileName, dirPathC.getObjectID(), false);
+    verifyKeyInOpenFileTable(openFileTable, clientID, fileName,
+            dirPathC.getObjectID(), true);
 
     ozoneBucket.deleteKey(key);
 
     // get deleted key
     try {
       ozoneBucket.getKey(key);
-      fail("Should throw exception as file not exists!");
+      fail("Should throw exception as fileName not exists!");
     } catch (OMException ome) {
       // expected
       assertEquals(ome.getResult(), OMException.ResultCodes.KEY_NOT_FOUND);
     }
 
     // after key delete
-    verifyKeyInFileTable(keyTable, file, dirPathC.getObjectID(), true);
-    verifyKeyInOpenFileTable(openKeyTable, file, dirPathC.getObjectID(),
-            true);
+    verifyKeyInFileTable(fileTable, fileName, dirPathC.getObjectID(), true);
+    verifyKeyInOpenFileTable(openFileTable, clientID, fileName,
+            dirPathC.getObjectID(), true);
   }
 
-  private OmDirectoryInfo getDirInfo(String volumeName, String bucketName,
-      String parentKey) throws Exception {
+  private OmDirectoryInfo getDirInfo(String parentKey) throws Exception {
     OMMetadataManager omMetadataManager =
             cluster.getOzoneManager().getMetadataManager();
     long bucketId = TestOMRequestUtils.getBucketId(volumeName, bucketName,
@@ -238,51 +285,38 @@ public class TestObjectStoreV1 {
 
   private void verifyKeyInFileTable(Table<String, OmKeyInfo> fileTable,
       String fileName, long parentID, boolean isEmpty) throws IOException {
-    TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> iterator
-            = fileTable.iterator();
 
+    String dbFileKey = parentID + OM_KEY_PREFIX + fileName;
+    OmKeyInfo omKeyInfo = fileTable.get(dbFileKey);
     if (isEmpty) {
-      Assert.assertTrue("Table is not empty!", fileTable.isEmpty());
+      Assert.assertNull("Table is not empty!", omKeyInfo);
     } else {
-      Assert.assertFalse("Table is empty!", fileTable.isEmpty());
-      while (iterator.hasNext()) {
-        Table.KeyValue<String, OmKeyInfo> next = iterator.next();
-        Assert.assertEquals("Invalid Key: " + next.getKey(),
-                parentID + "/" + fileName, next.getKey());
-        OmKeyInfo omKeyInfo = next.getValue();
-        Assert.assertEquals("Invalid Key", fileName,
-                omKeyInfo.getFileName());
-        Assert.assertEquals("Invalid Key", fileName,
-                omKeyInfo.getKeyName());
-        Assert.assertEquals("Invalid Key", parentID,
-                omKeyInfo.getParentObjectID());
-      }
+      Assert.assertNotNull("Table is empty!", omKeyInfo);
+      // used startsWith because the key format is,
+      // <parentID>/fileName/<clientID> and clientID is not visible.
+      Assert.assertEquals("Invalid Key: " + omKeyInfo.getObjectInfo(),
+              omKeyInfo.getKeyName(), fileName);
+      Assert.assertEquals("Invalid Key", parentID,
+              omKeyInfo.getParentObjectID());
     }
   }
 
   private void verifyKeyInOpenFileTable(Table<String, OmKeyInfo> openFileTable,
-      String fileName, long parentID, boolean isEmpty) throws IOException {
-    TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> iterator
-            = openFileTable.iterator();
-
+      long clientID, String fileName, long parentID, boolean isEmpty)
+          throws IOException {
+    String dbOpenFileKey =
+            parentID + OM_KEY_PREFIX + fileName + OM_KEY_PREFIX + clientID;
+    OmKeyInfo omKeyInfo = openFileTable.get(dbOpenFileKey);
     if (isEmpty) {
-      Assert.assertTrue("Table is not empty!", openFileTable.isEmpty());
+      Assert.assertNull("Table is not empty!", omKeyInfo);
     } else {
-      Assert.assertFalse("Table is empty!", openFileTable.isEmpty());
-      while (iterator.hasNext()) {
-        Table.KeyValue<String, OmKeyInfo> next = iterator.next();
-        // used startsWith because the key format is,
-        // <parentID>/fileName/<clientID> and clientID is not visible.
-        Assert.assertTrue("Invalid Key: " + next.getKey(),
-                next.getKey().startsWith(parentID + "/" + fileName));
-        OmKeyInfo omKeyInfo = next.getValue();
-        Assert.assertEquals("Invalid Key", fileName,
-                omKeyInfo.getFileName());
-        Assert.assertEquals("Invalid Key", fileName,
-                omKeyInfo.getKeyName());
-        Assert.assertEquals("Invalid Key", parentID,
-                omKeyInfo.getParentObjectID());
-      }
+      Assert.assertNotNull("Table is empty!", omKeyInfo);
+      // used startsWith because the key format is,
+      // <parentID>/fileName/<clientID> and clientID is not visible.
+      Assert.assertEquals("Invalid Key: " + omKeyInfo.getObjectInfo(),
+              omKeyInfo.getKeyName(), fileName);
+      Assert.assertEquals("Invalid Key", parentID,
+              omKeyInfo.getParentObjectID());
     }
   }
 
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 db28ff7..055ab13 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
@@ -31,7 +31,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -2320,6 +2319,7 @@ public class KeyManagerImpl implements KeyManager {
     return fileStatusList;
   }
 
+  @SuppressWarnings("methodlength")
   public List<OzoneFileStatus> listStatusV1(OmKeyArgs args, boolean recursive,
       String startKey, long numEntries, String clientAddress)
           throws IOException {
@@ -2327,10 +2327,32 @@ public class KeyManagerImpl implements KeyManager {
 
     // unsorted OMKeyInfo list contains combine results from TableCache and DB.
     List<OzoneFileStatus> fileStatusFinalList = new ArrayList<>();
-    LinkedHashSet<OzoneFileStatus> fileStatusList = new LinkedHashSet<>();
+
     if (numEntries <= 0) {
       return fileStatusFinalList;
     }
+
+    /**
+     * A map sorted by OmKey to combine results from TableCache and DB for
+     * each entity - Dir & File.
+     *
+     * Two separate maps are required because the order of seek -> (1)Seek
+     * files in fileTable (2)Seek dirs in dirTable.
+     *
+     * StartKey should be added to the final listStatuses, so if we combine
+     * files and dirs into a single map then directory with lower precedence
+     * will appear at the top of the list even if the startKey is given as
+     * fileName.
+     *
+     * For example, startKey="a/file1". As per the seek order, first fetches
+     * all the files and then it will start seeking all the directories.
+     * Assume a directory name exists "a/b". With one map, the sorted list will
+     * be ["a/b", "a/file1"]. But the expected list is: ["a/file1", "a/b"],
+     * startKey element should always be at the top of the listStatuses.
+     */
+    TreeMap<String, OzoneFileStatus> cacheFileMap = new TreeMap<>();
+    TreeMap<String, OzoneFileStatus> cacheDirMap = new TreeMap<>();
+
     String volumeName = args.getVolumeName();
     String bucketName = args.getBucketName();
     String keyName = args.getKeyName();
@@ -2373,12 +2395,12 @@ public class KeyManagerImpl implements KeyManager {
         seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
         seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
 
-        // Order of seek -> (1)Seek dirs in dirTable (2)Seek files in fileTable
+        // Order of seek -> (1)Seek files in fileTable (2)Seek dirs in dirTable
         // 1. Seek the given key in key table.
-        countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB,
+        countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
                 prefixPath, prefixKeyInDB, startKey, countEntries, numEntries);
         // 2. Seek the given key in dir table.
-        getDirectories(fileStatusList, seekDirInDB, prefixPath, prefixKeyInDB,
+        getDirectories(cacheDirMap, seekDirInDB, prefixPath, prefixKeyInDB,
                 startKey, countEntries, numEntries, volumeName, bucketName,
                 recursive);
       } else {
@@ -2420,7 +2442,7 @@ public class KeyManagerImpl implements KeyManager {
             // dirTable. So, its not required to search again in the fileTable.
 
             // Seek the given key in dirTable.
-            getDirectories(fileStatusList, seekDirInDB, prefixPath,
+            getDirectories(cacheDirMap, seekDirInDB, prefixPath,
                     prefixKeyInDB, startKey, countEntries, numEntries,
                     volumeName, bucketName, recursive);
           } else {
@@ -2430,11 +2452,11 @@ public class KeyManagerImpl implements KeyManager {
             seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
 
             // 1. Seek the given key in key table.
-            countEntries = getFilesFromDirectory(fileStatusList, seekFileInDB,
+            countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
                     prefixPath, prefixKeyInDB, startKey, countEntries,
                     numEntries);
             // 2. Seek the given key in dir table.
-            getDirectories(fileStatusList, seekDirInDB, prefixPath,
+            getDirectories(cacheDirMap, seekDirInDB, prefixPath,
                     prefixKeyInDB, startKey, countEntries, numEntries,
                     volumeName, bucketName, recursive);
           }
@@ -2451,12 +2473,16 @@ public class KeyManagerImpl implements KeyManager {
       metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
               bucketName);
     }
-    List<OmKeyInfo> keyInfoList = new ArrayList<>(fileStatusList.size());
-    for (OzoneFileStatus fileStatus : fileStatusList) {
-      if (fileStatus.isFile()) {
-        keyInfoList.add(fileStatus.getKeyInfo());
-      }
+
+    List<OmKeyInfo> keyInfoList = new ArrayList<>();
+    for (OzoneFileStatus fileStatus : cacheFileMap.values()) {
+      fileStatusFinalList.add(fileStatus);
+      keyInfoList.add(fileStatus.getKeyInfo());
+    }
+    for (OzoneFileStatus fileStatus : cacheDirMap.values()) {
+      fileStatusFinalList.add(fileStatus);
     }
+
     // refreshPipeline flag check has been removed as part of
     // https://issues.apache.org/jira/browse/HDDS-3658.
     // Please refer this jira for more details.
@@ -2464,20 +2490,23 @@ public class KeyManagerImpl implements KeyManager {
     if (args.getSortDatanodes()) {
       sortDatanodes(clientAddress, keyInfoList.toArray(new OmKeyInfo[0]));
     }
-    fileStatusFinalList.addAll(fileStatusList);
     return fileStatusFinalList;
   }
 
   @SuppressWarnings("parameternumber")
-  protected int getDirectories(Set<OzoneFileStatus> fileStatusList,
+  protected int getDirectories(
+      TreeMap<String, OzoneFileStatus> cacheKeyMap,
       String seekDirInDB, String prefixPath, long prefixKeyInDB,
       String startKey, int countEntries, long numEntries, String volumeName,
       String bucketName, boolean recursive) throws IOException {
 
+    // A set to keep track of keys deleted in cache but not flushed to DB.
+    Set<String> deletedKeySet = new TreeSet<>();
+
     Table dirTable = metadataManager.getDirectoryTable();
-    countEntries = listStatusFindDirsInTableCache(fileStatusList, dirTable,
+    countEntries = listStatusFindDirsInTableCache(cacheKeyMap, dirTable,
             prefixKeyInDB, seekDirInDB, prefixPath, startKey, volumeName,
-            bucketName, countEntries, numEntries);
+            bucketName, countEntries, numEntries, deletedKeySet);
     TableIterator<String, ? extends Table.KeyValue<String, OmDirectoryInfo>>
             iterator = dirTable.iterator();
 
@@ -2485,6 +2514,11 @@ public class KeyManagerImpl implements KeyManager {
 
     while (iterator.hasNext() && numEntries - countEntries > 0) {
       OmDirectoryInfo dirInfo = iterator.value().getValue();
+      if (deletedKeySet.contains(dirInfo.getPath())) {
+        iterator.next(); // move to next entry in the table
+        // entry is actually deleted in cache and can exists in DB
+        continue;
+      }
       if (!OMFileRequest.isImmediateChild(dirInfo.getParentObjectID(),
               prefixKeyInDB)) {
         break;
@@ -2496,7 +2530,7 @@ public class KeyManagerImpl implements KeyManager {
                 dirInfo.getName());
         OmKeyInfo omKeyInfo = OMFileRequest.getOmKeyInfo(volumeName,
                 bucketName, dirInfo, dirName);
-        fileStatusList.add(new OzoneFileStatus(omKeyInfo, scmBlockSize,
+        cacheKeyMap.put(dirName, new OzoneFileStatus(omKeyInfo, scmBlockSize,
                 true));
         countEntries++;
       }
@@ -2507,20 +2541,28 @@ public class KeyManagerImpl implements KeyManager {
     return countEntries;
   }
 
-  private int getFilesFromDirectory(Set<OzoneFileStatus> fileStatusList,
+  private int getFilesFromDirectory(
+      TreeMap<String, OzoneFileStatus> cacheKeyMap,
       String seekKeyInDB, String prefixKeyPath, long prefixKeyInDB,
       String startKey, int countEntries, long numEntries) throws IOException {
 
+    // A set to keep track of keys deleted in cache but not flushed to DB.
+    Set<String> deletedKeySet = new TreeSet<>();
+
     Table<String, OmKeyInfo> keyTable = metadataManager.getKeyTable();
-    countEntries = listStatusFindFilesInTableCache(fileStatusList, keyTable,
+    countEntries = listStatusFindFilesInTableCache(cacheKeyMap, keyTable,
             prefixKeyInDB, seekKeyInDB, prefixKeyPath, startKey,
-            countEntries, numEntries);
+            countEntries, numEntries, deletedKeySet);
     TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
             iterator = keyTable.iterator();
     iterator.seek(seekKeyInDB);
     while (iterator.hasNext() && numEntries - countEntries > 0) {
       OmKeyInfo keyInfo = iterator.value().getValue();
-
+      if (deletedKeySet.contains(keyInfo.getPath())) {
+        iterator.next(); // move to next entry in the table
+        // entry is actually deleted in cache and can exists in DB
+        continue;
+      }
       if (!OMFileRequest.isImmediateChild(keyInfo.getParentObjectID(),
               prefixKeyInDB)) {
         break;
@@ -2530,7 +2572,8 @@ public class KeyManagerImpl implements KeyManager {
       String fullKeyPath = OMFileRequest.getAbsolutePath(prefixKeyPath,
               keyInfo.getKeyName());
       keyInfo.setKeyName(fullKeyPath);
-      fileStatusList.add(new OzoneFileStatus(keyInfo, scmBlockSize, false));
+      cacheKeyMap.put(fullKeyPath,
+              new OzoneFileStatus(keyInfo, scmBlockSize, false));
       countEntries++;
       iterator.next(); // move to next entry in the table
     }
@@ -2542,10 +2585,10 @@ public class KeyManagerImpl implements KeyManager {
    */
   @SuppressWarnings("parameternumber")
   private int listStatusFindFilesInTableCache(
-          Set<OzoneFileStatus> fileStatusList, Table<String,
+          TreeMap<String, OzoneFileStatus> cacheKeyMap, Table<String,
           OmKeyInfo> keyTable, long prefixKeyInDB, String seekKeyInDB,
           String prefixKeyPath, String startKey, int countEntries,
-          long numEntries) {
+          long numEntries, Set<String> deletedKeySet) {
 
     Iterator<Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>>>
             cacheIter = keyTable.cacheIterator();
@@ -2558,6 +2601,7 @@ public class KeyManagerImpl implements KeyManager {
       OmKeyInfo cacheOmKeyInfo = entry.getValue().getCacheValue();
       // cacheOmKeyInfo is null if an entry is deleted in cache
       if(cacheOmKeyInfo == null){
+        deletedKeySet.add(cacheKey);
         continue;
       }
 
@@ -2571,7 +2615,7 @@ public class KeyManagerImpl implements KeyManager {
               omKeyInfo.getKeyName());
       omKeyInfo.setKeyName(fullKeyPath);
 
-      countEntries = addKeyInfoToFileStatusList(fileStatusList, prefixKeyInDB,
+      countEntries = addKeyInfoToFileStatusList(cacheKeyMap, prefixKeyInDB,
               seekKeyInDB, startKey, countEntries, cacheKey, omKeyInfo,
               false);
     }
@@ -2583,10 +2627,11 @@ public class KeyManagerImpl implements KeyManager {
    */
   @SuppressWarnings("parameternumber")
   private int listStatusFindDirsInTableCache(
-          Set<OzoneFileStatus> fileStatusList, Table<String,
+          TreeMap<String, OzoneFileStatus> cacheKeyMap, Table<String,
           OmDirectoryInfo> dirTable, long prefixKeyInDB, String seekKeyInDB,
           String prefixKeyPath, String startKey, String volumeName,
-          String bucketName, int countEntries, long numEntries) {
+          String bucketName, int countEntries, long numEntries,
+          Set<String> deletedKeySet) {
 
     Iterator<Map.Entry<CacheKey<String>, CacheValue<OmDirectoryInfo>>>
             cacheIter = dirTable.cacheIterator();
@@ -2599,7 +2644,9 @@ public class KeyManagerImpl implements KeyManager {
               cacheIter.next();
       String cacheKey = entry.getKey().getCacheKey();
       OmDirectoryInfo cacheOmDirInfo = entry.getValue().getCacheValue();
+      // cacheOmKeyInfo is null if an entry is deleted in cache
       if(cacheOmDirInfo == null){
+        deletedKeySet.add(cacheKey);
         continue;
       }
       String fullDirPath = OMFileRequest.getAbsolutePath(prefixKeyPath,
@@ -2607,7 +2654,7 @@ public class KeyManagerImpl implements KeyManager {
       OmKeyInfo cacheDirKeyInfo = OMFileRequest.getOmKeyInfo(volumeName,
               bucketName, cacheOmDirInfo, fullDirPath);
 
-      countEntries = addKeyInfoToFileStatusList(fileStatusList, prefixKeyInDB,
+      countEntries = addKeyInfoToFileStatusList(cacheKeyMap, prefixKeyInDB,
               seekKeyInDB, startKey, countEntries, cacheKey, cacheDirKeyInfo,
               true);
     }
@@ -2615,7 +2662,8 @@ public class KeyManagerImpl implements KeyManager {
   }
 
   @SuppressWarnings("parameternumber")
-  private int addKeyInfoToFileStatusList(Set<OzoneFileStatus> fileStatusList,
+  private int addKeyInfoToFileStatusList(
+      TreeMap<String, OzoneFileStatus> cacheKeyMap,
       long prefixKeyInDB, String seekKeyInDB, String startKey,
       int countEntries, String cacheKey, OmKeyInfo cacheOmKeyInfo,
       boolean isDirectory) {
@@ -2627,7 +2675,7 @@ public class KeyManagerImpl implements KeyManager {
       if (cacheKey.startsWith(seekKeyInDB)) {
         OzoneFileStatus fileStatus = new OzoneFileStatus(cacheOmKeyInfo,
                 scmBlockSize, isDirectory);
-        fileStatusList.add(fileStatus);
+        cacheKeyMap.put(cacheOmKeyInfo.getKeyName(), fileStatus);
         countEntries++;
       }
     } else {
@@ -2641,7 +2689,7 @@ public class KeyManagerImpl implements KeyManager {
               cacheKey.compareTo(seekKeyInDB) >= 0) {
         OzoneFileStatus fileStatus = new OzoneFileStatus(cacheOmKeyInfo,
                 scmBlockSize, isDirectory);
-        fileStatusList.add(fileStatus);
+        cacheKeyMap.put(cacheOmKeyInfo.getKeyName(), fileStatus);
         countEntries++;
       }
     }
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java
index af3bc82..af5c4df 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java
@@ -31,6 +31,7 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
 import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
+import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
 import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
 import org.apache.hadoop.ozone.om.request.file.OMFileRequest;
 import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
@@ -119,6 +120,11 @@ public class OMKeyDeleteRequestV1 extends OMKeyDeleteRequest {
       }
 
       OmKeyInfo omKeyInfo = keyStatus.getKeyInfo();
+      // New key format for the fileTable & dirTable.
+      // For example, the user given key path is '/a/b/c/d/e/file1', then in DB
+      // keyName field stores only the leaf node name, which is 'file1'.
+      String fileName = OzoneFSUtils.getFileName(keyName);
+      omKeyInfo.setKeyName(fileName);
 
       // Set the UpdateID to current transactionLogIndex
       omKeyInfo.setUpdateID(trxnLogIndex, ozoneManager.isRatisEnabled());

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org