You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/05 08:22:35 UTC

[GitHub] [ozone] bharatviswa504 commented on a change in pull request #2369: HDDS-5349. Avoid usage of locks in listStatus.

bharatviswa504 commented on a change in pull request #2369:
URL: https://github.com/apache/ozone/pull/2369#discussion_r683236929



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2483,159 +2484,276 @@ private void listStatusFindKeyInTableCache(
     int countEntries = 0;
 
     // TODO: recursive flag=true will be handled in HDDS-4360 jira.
-    metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
-            bucketName);
-    try {
-      if (Strings.isNullOrEmpty(startKey)) {
-        OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
-        if (fileStatus.isFile()) {
-          return Collections.singletonList(fileStatus);
-        }
 
-        // Not required to search in DeletedTable because all the deleted
-        // keys will be marked directly in dirTable or in keyTable by
-        // breaking the pointer to its sub-dirs and sub-files. So, there is no
-        // issue of inconsistency.
-
-        /*
-         * keyName is a directory.
-         * Say, "/a" is the dir name and its objectID is 1024, then seek
-         * will be doing with "1024/" to get all immediate descendants.
-         */
-        if (fileStatus.getKeyInfo() != null) {
-          prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
-        } else {
-          // list root directory.
-          String bucketKey = metadataManager.getBucketKey(volumeName,
-                  bucketName);
-          OmBucketInfo omBucketInfo =
-                  metadataManager.getBucketTable().get(bucketKey);
-          prefixKeyInDB = omBucketInfo.getObjectID();
-        }
-        seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
-        seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
-
-        // Order of seek -> (1)Seek files in fileTable (2)Seek dirs in dirTable
-        // 1. Seek the given key in key table.
-        countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
-                prefixPath, prefixKeyInDB, startKey, countEntries, numEntries);
-        // 2. Seek the given key in dir table.
-        getDirectories(cacheDirMap, seekDirInDB, prefixPath, prefixKeyInDB,
-                startKey, countEntries, numEntries, volumeName, bucketName,
-                recursive);
-      } else {
-        /*
-         * startKey will be used in iterator seek and sets the beginning point
-         * for key traversal.
-         * keyName will be used as parentID where the user has requested to
-         * list the keys from.
-         *
-         * When recursive flag=false, parentID won't change between two pages.
-         * For example: OM has a namespace like,
-         *    /a/1...1M files and /a/b/1...1M files.
-         *    /a/1...1M directories and /a/b/1...1M directories.
-         * Listing "/a", will always have the parentID as "a" irrespective of
-         * the startKey value.
-         */
-
-        // Check startKey is an immediate child of keyName. For example,
-        // keyName=/a/ and expected startKey=/a/b. startKey can't be /xyz/b.
-        if (StringUtils.isNotBlank(keyName) &&
-                !OzoneFSUtils.isImmediateChild(keyName, startKey)) {
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("StartKey {} is not an immediate child of keyName {}. " +
-                    "Returns empty list", startKey, keyName);
-          }
-          return Collections.emptyList();
-        }
 
-        // assign startKeyPath if prefixPath is empty string.
-        if (StringUtils.isBlank(prefixPath)) {
-          prefixPath = OzoneFSUtils.getParentDir(startKey);
+    if (Strings.isNullOrEmpty(startKey)) {
+      OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
+      if (fileStatus.isFile()) {
+        return Collections.singletonList(fileStatus);
+      }
+
+      // Not required to search in DeletedTable because all the deleted
+      // keys will be marked directly in dirTable or in keyTable by
+      // breaking the pointer to its sub-dirs and sub-files. So, there is no
+      // issue of inconsistency.
+
+      /*
+       * keyName is a directory.
+       * Say, "/a" is the dir name and its objectID is 1024, then seek
+       * will be doing with "1024/" to get all immediate descendants.
+       */
+      if (fileStatus.getKeyInfo() != null) {
+        prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
+      } else {
+        // list root directory.
+        String bucketKey = metadataManager.getBucketKey(volumeName, bucketName);
+        OmBucketInfo omBucketInfo =
+            metadataManager.getBucketTable().get(bucketKey);
+        prefixKeyInDB = omBucketInfo.getObjectID();
+      }
+      seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+      seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+
+      // Order of seek ->
+      // (1)Seek files in fileTable
+      // (2)Seek dirs in dirTable
+
+      Set<String> deletedKeySet = new TreeSet<>();
+      TreeMap<String, OzoneFileStatus> tempCacheDirMap = new TreeMap<>();
+
+      // Get files, dirs and marked for delete entries from cache.
+      countEntries = getFilesAndDirsFromCache(volumeName, bucketName,
+          cacheFileMap, tempCacheDirMap, deletedKeySet, prefixKeyInDB,
+          seekFileInDB, seekDirInDB, prefixPath, startKey, countEntries,
+          numEntries);
+
+      countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
+          prefixPath, prefixKeyInDB, countEntries, numEntries, deletedKeySet);
+
+      // Now first add all entries in the dir cache, if numEntries required
+      // is less than countEntries.
+      for (Map.Entry<String, OzoneFileStatus> dirEntry :
+          tempCacheDirMap.entrySet()) {
+        if (countEntries < numEntries) {
+          cacheDirMap.put(dirEntry.getKey(), dirEntry.getValue());
+          countEntries++;
         }
+      }
 
-        OzoneFileStatus fileStatusInfo = getOzoneFileStatusFSO(volumeName,
-                bucketName, startKey, false, null,
-                args.getLatestVersionLocation(), true);
+      // 2. Seek the given key in dir table.
+      if (countEntries < numEntries) {
+        getDirectories(cacheDirMap, seekDirInDB, prefixPath,
+            prefixKeyInDB, countEntries, numEntries, recursive,
+            volumeName, bucketName, deletedKeySet);
+      }
 
-        if (fileStatusInfo != null) {
-          prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
-          if(fileStatusInfo.isDirectory()){
-            seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
-                    fileStatusInfo.getKeyInfo().getFileName());
+    } else {
+      /*
+       * startKey will be used in iterator seek and sets the beginning point
+       * for key traversal.
+       * keyName will be used as parentID where the user has requested to
+       * list the keys from.
+       *
+       * When recursive flag=false, parentID won't change between two pages.
+       * For example: OM has a namespace like,
+       *    /a/1...1M files and /a/b/1...1M files.
+       *    /a/1...1M directories and /a/b/1...1M directories.
+       * Listing "/a", will always have the parentID as "a" irrespective of
+       * the startKey value.
+       */
+
+      // Check startKey is an immediate child of keyName. For example,
+      // keyName=/a/ and expected startKey=/a/b. startKey can't be /xyz/b.
+      if (StringUtils.isNotBlank(keyName) &&
+          !OzoneFSUtils.isImmediateChild(keyName, startKey)) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("StartKey {} is not an immediate child of keyName {}. " +
+              "Returns empty list", startKey, keyName);
+        }
+        return Collections.emptyList();
+      }
+
+      // assign startKeyPath if prefixPath is empty string.
+      if (StringUtils.isBlank(prefixPath)) {
+        prefixPath = OzoneFSUtils.getParentDir(startKey);
+      }
+
+      OzoneFileStatus fileStatusInfo = getOzoneFileStatusFSO(volumeName,
+          bucketName, startKey, false, null,
+          args.getLatestVersionLocation(), true);
+
+      if (fileStatusInfo != null) {
+        prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
+        if(fileStatusInfo.isDirectory()){
+          seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+              fileStatusInfo.getKeyInfo().getFileName());
+
+          // Order of seek -> (1) Seek dirs only in dirTable. In OM, always
+          // the order of search is, first seek into fileTable and then
+          // dirTable. So, its not required to search again in the fileTable.
+
+          // Seek the given key in dirTable.
+          metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
+                bucketName);
+
+          TreeSet<String> deletedKeySet = new TreeSet<>();
+          TreeMap<String, OzoneFileStatus> tempCacheDirMap = new TreeMap<>();
+          try {
+            listStatusFindDirsInTableCache(tempCacheDirMap,
+                metadataManager.getDirectoryTable(),
+                prefixKeyInDB, seekDirInDB, prefixPath, startKey, volumeName,
+                bucketName, countEntries, numEntries, deletedKeySet);
+          } finally {
+            metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+                bucketName);
+          }
 
-            // Order of seek -> (1) Seek dirs only in dirTable. In OM, always
-            // the order of search is, first seek into fileTable and then
-            // dirTable. So, its not required to search again in the fileTable.
+          // Now first add all entries in the dir cache, if numEntries required
+          // is less than countEntries.
+          for (Map.Entry<String, OzoneFileStatus> dirEntry :
+              tempCacheDirMap.entrySet()) {
+            if (countEntries < numEntries) {
+              cacheDirMap.put(dirEntry.getKey(), dirEntry.getValue());
+              countEntries++;
+            }
+          }
 
-            // Seek the given key in dirTable.
-            getDirectories(cacheDirMap, seekDirInDB, prefixPath,
-                    prefixKeyInDB, startKey, countEntries, numEntries,
-                    volumeName, bucketName, recursive);
-          } else {
-            seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
-                    fileStatusInfo.getKeyInfo().getFileName());
-            // begins from the first sub-dir under the parent dir
-            seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
-
-            // 1. Seek the given key in key table.
-            countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
-                    prefixPath, prefixKeyInDB, startKey, countEntries,
-                    numEntries);
-            // 2. Seek the given key in dir table.
+          if (countEntries < numEntries) {
             getDirectories(cacheDirMap, seekDirInDB, prefixPath,
-                    prefixKeyInDB, startKey, countEntries, numEntries,
-                    volumeName, bucketName, recursive);
+                prefixKeyInDB, countEntries, numEntries, recursive,
+                volumeName, bucketName, deletedKeySet);
           }
         } else {
-          // TODO: HDDS-4364: startKey can be a non-existed key
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("StartKey {} is a non-existed key and returning empty " +
-                    "list", startKey);
+          seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+              fileStatusInfo.getKeyInfo().getFileName());
+          // begins from the first sub-dir under the parent dir
+          seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+
+          TreeSet<String> deletedKeySet = new TreeSet<>();
+          TreeMap<String, OzoneFileStatus> tempCacheDirMap = new TreeMap<>();
+
+          // Get files, dirs and marked for delete entries from cache.
+          countEntries = getFilesAndDirsFromCache(volumeName, bucketName,
+              cacheFileMap, tempCacheDirMap, deletedKeySet, prefixKeyInDB,
+              seekFileInDB, seekDirInDB, prefixPath, startKey, countEntries,
+              numEntries);
+
+          // 1. Seek the given key in key table.
+          countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
+              prefixPath, prefixKeyInDB, countEntries, numEntries,
+              deletedKeySet);
+
+          // Now first add all entries in the dir cache, if numEntries required
+          // is less than countEntries.
+          for (Map.Entry<String, OzoneFileStatus> dirEntry :
+              tempCacheDirMap.entrySet()) {
+            if (countEntries < numEntries) {
+              cacheDirMap.put(dirEntry.getKey(), dirEntry.getValue());
+              countEntries++;
+            }
+          }
+
+          // 2. Seek the given key in dir table.
+          if (countEntries < numEntries) {
+            getDirectories(cacheDirMap, seekDirInDB, prefixPath,
+                prefixKeyInDB, countEntries, numEntries, recursive,
+                volumeName, bucketName, deletedKeySet);
           }
-          return Collections.emptyList();
         }
+      } else {
+        // TODO: HDDS-4364: startKey can be a non-existed key
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("StartKey {} is a non-existed key and returning empty " +
+                    "list", startKey);
+        }
+        return Collections.emptyList();
       }
-    } finally {
-      metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
-              bucketName);
     }
 
+    return buildFinalStatusList(cacheFileMap, cacheDirMap, args, clientAddress);
+
+  }
+
+  /**
+   * Build final OzoneFileStatus list to be returned to client.
+   * @throws IOException
+   */
+  private List<OzoneFileStatus> buildFinalStatusList(
+      Map<String, OzoneFileStatus> cacheFileMap,
+      Map<String, OzoneFileStatus> cacheDirMap, OmKeyArgs omKeyArgs,
+      String clientAddress)
+      throws IOException {
     List<OmKeyInfo> keyInfoList = new ArrayList<>();
+    List<OzoneFileStatus> fileStatusFinalList = new ArrayList<>();
+
     for (OzoneFileStatus fileStatus : cacheFileMap.values()) {
       fileStatusFinalList.add(fileStatus);
       keyInfoList.add(fileStatus.getKeyInfo());
     }
+
     for (OzoneFileStatus fileStatus : cacheDirMap.values()) {
       fileStatusFinalList.add(fileStatus);
     }
-    if (args.getLatestVersionLocation()) {
+
+    if (omKeyArgs.getLatestVersionLocation()) {
       slimLocationVersion(keyInfoList.toArray(new OmKeyInfo[0]));
     }
     // refreshPipeline flag check has been removed as part of
     // https://issues.apache.org/jira/browse/HDDS-3658.
     // Please refer this jira for more details.
     refreshPipeline(keyInfoList);
-    if (args.getSortDatanodes()) {
+
+    if (omKeyArgs.getSortDatanodes()) {
       sortDatanodes(clientAddress, keyInfoList.toArray(new OmKeyInfo[0]));
     }
+
     return fileStatusFinalList;
   }
 
+  /***
+   * Build files, directories and marked for deleted entries from dir/file
+   * cache.
+   */
+  @SuppressWarnings("parameternumber")
+  private int getFilesAndDirsFromCache(String volumeName, String bucketName,

Review comment:
       Done

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2483,159 +2484,276 @@ private void listStatusFindKeyInTableCache(
     int countEntries = 0;
 
     // TODO: recursive flag=true will be handled in HDDS-4360 jira.
-    metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
-            bucketName);
-    try {
-      if (Strings.isNullOrEmpty(startKey)) {
-        OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
-        if (fileStatus.isFile()) {
-          return Collections.singletonList(fileStatus);
-        }
 
-        // Not required to search in DeletedTable because all the deleted
-        // keys will be marked directly in dirTable or in keyTable by
-        // breaking the pointer to its sub-dirs and sub-files. So, there is no
-        // issue of inconsistency.
-
-        /*
-         * keyName is a directory.
-         * Say, "/a" is the dir name and its objectID is 1024, then seek
-         * will be doing with "1024/" to get all immediate descendants.
-         */
-        if (fileStatus.getKeyInfo() != null) {
-          prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
-        } else {
-          // list root directory.
-          String bucketKey = metadataManager.getBucketKey(volumeName,
-                  bucketName);
-          OmBucketInfo omBucketInfo =
-                  metadataManager.getBucketTable().get(bucketKey);
-          prefixKeyInDB = omBucketInfo.getObjectID();
-        }
-        seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
-        seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
-
-        // Order of seek -> (1)Seek files in fileTable (2)Seek dirs in dirTable
-        // 1. Seek the given key in key table.
-        countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
-                prefixPath, prefixKeyInDB, startKey, countEntries, numEntries);
-        // 2. Seek the given key in dir table.
-        getDirectories(cacheDirMap, seekDirInDB, prefixPath, prefixKeyInDB,
-                startKey, countEntries, numEntries, volumeName, bucketName,
-                recursive);
-      } else {
-        /*
-         * startKey will be used in iterator seek and sets the beginning point
-         * for key traversal.
-         * keyName will be used as parentID where the user has requested to
-         * list the keys from.
-         *
-         * When recursive flag=false, parentID won't change between two pages.
-         * For example: OM has a namespace like,
-         *    /a/1...1M files and /a/b/1...1M files.
-         *    /a/1...1M directories and /a/b/1...1M directories.
-         * Listing "/a", will always have the parentID as "a" irrespective of
-         * the startKey value.
-         */
-
-        // Check startKey is an immediate child of keyName. For example,
-        // keyName=/a/ and expected startKey=/a/b. startKey can't be /xyz/b.
-        if (StringUtils.isNotBlank(keyName) &&
-                !OzoneFSUtils.isImmediateChild(keyName, startKey)) {
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("StartKey {} is not an immediate child of keyName {}. " +
-                    "Returns empty list", startKey, keyName);
-          }
-          return Collections.emptyList();
-        }
 
-        // assign startKeyPath if prefixPath is empty string.
-        if (StringUtils.isBlank(prefixPath)) {
-          prefixPath = OzoneFSUtils.getParentDir(startKey);
+    if (Strings.isNullOrEmpty(startKey)) {
+      OzoneFileStatus fileStatus = getFileStatus(args, clientAddress);
+      if (fileStatus.isFile()) {
+        return Collections.singletonList(fileStatus);
+      }
+
+      // Not required to search in DeletedTable because all the deleted
+      // keys will be marked directly in dirTable or in keyTable by
+      // breaking the pointer to its sub-dirs and sub-files. So, there is no
+      // issue of inconsistency.
+
+      /*
+       * keyName is a directory.
+       * Say, "/a" is the dir name and its objectID is 1024, then seek
+       * will be doing with "1024/" to get all immediate descendants.
+       */
+      if (fileStatus.getKeyInfo() != null) {
+        prefixKeyInDB = fileStatus.getKeyInfo().getObjectID();
+      } else {
+        // list root directory.
+        String bucketKey = metadataManager.getBucketKey(volumeName, bucketName);
+        OmBucketInfo omBucketInfo =
+            metadataManager.getBucketTable().get(bucketKey);
+        prefixKeyInDB = omBucketInfo.getObjectID();
+      }
+      seekFileInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+      seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB, "");
+
+      // Order of seek ->
+      // (1)Seek files in fileTable
+      // (2)Seek dirs in dirTable
+
+      Set<String> deletedKeySet = new TreeSet<>();
+      TreeMap<String, OzoneFileStatus> tempCacheDirMap = new TreeMap<>();
+
+      // Get files, dirs and marked for delete entries from cache.
+      countEntries = getFilesAndDirsFromCache(volumeName, bucketName,
+          cacheFileMap, tempCacheDirMap, deletedKeySet, prefixKeyInDB,
+          seekFileInDB, seekDirInDB, prefixPath, startKey, countEntries,
+          numEntries);
+
+      countEntries = getFilesFromDirectory(cacheFileMap, seekFileInDB,
+          prefixPath, prefixKeyInDB, countEntries, numEntries, deletedKeySet);
+
+      // Now first add all entries in the dir cache, if numEntries required
+      // is less than countEntries.
+      for (Map.Entry<String, OzoneFileStatus> dirEntry :
+          tempCacheDirMap.entrySet()) {
+        if (countEntries < numEntries) {
+          cacheDirMap.put(dirEntry.getKey(), dirEntry.getValue());
+          countEntries++;
         }
+      }
 
-        OzoneFileStatus fileStatusInfo = getOzoneFileStatusFSO(volumeName,
-                bucketName, startKey, false, null,
-                args.getLatestVersionLocation(), true);
+      // 2. Seek the given key in dir table.
+      if (countEntries < numEntries) {
+        getDirectories(cacheDirMap, seekDirInDB, prefixPath,
+            prefixKeyInDB, countEntries, numEntries, recursive,
+            volumeName, bucketName, deletedKeySet);
+      }
 
-        if (fileStatusInfo != null) {
-          prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
-          if(fileStatusInfo.isDirectory()){
-            seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
-                    fileStatusInfo.getKeyInfo().getFileName());
+    } else {
+      /*
+       * startKey will be used in iterator seek and sets the beginning point
+       * for key traversal.
+       * keyName will be used as parentID where the user has requested to
+       * list the keys from.
+       *
+       * When recursive flag=false, parentID won't change between two pages.
+       * For example: OM has a namespace like,
+       *    /a/1...1M files and /a/b/1...1M files.
+       *    /a/1...1M directories and /a/b/1...1M directories.
+       * Listing "/a", will always have the parentID as "a" irrespective of
+       * the startKey value.
+       */
+
+      // Check startKey is an immediate child of keyName. For example,
+      // keyName=/a/ and expected startKey=/a/b. startKey can't be /xyz/b.
+      if (StringUtils.isNotBlank(keyName) &&
+          !OzoneFSUtils.isImmediateChild(keyName, startKey)) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("StartKey {} is not an immediate child of keyName {}. " +
+              "Returns empty list", startKey, keyName);
+        }
+        return Collections.emptyList();
+      }
+
+      // assign startKeyPath if prefixPath is empty string.
+      if (StringUtils.isBlank(prefixPath)) {
+        prefixPath = OzoneFSUtils.getParentDir(startKey);
+      }
+
+      OzoneFileStatus fileStatusInfo = getOzoneFileStatusFSO(volumeName,
+          bucketName, startKey, false, null,
+          args.getLatestVersionLocation(), true);
+
+      if (fileStatusInfo != null) {
+        prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
+        if(fileStatusInfo.isDirectory()){
+          seekDirInDB = metadataManager.getOzonePathKey(prefixKeyInDB,
+              fileStatusInfo.getKeyInfo().getFileName());
+
+          // Order of seek -> (1) Seek dirs only in dirTable. In OM, always
+          // the order of search is, first seek into fileTable and then
+          // dirTable. So, its not required to search again in the fileTable.
+
+          // Seek the given key in dirTable.
+          metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
+                bucketName);
+
+          TreeSet<String> deletedKeySet = new TreeSet<>();
+          TreeMap<String, OzoneFileStatus> tempCacheDirMap = new TreeMap<>();
+          try {
+            listStatusFindDirsInTableCache(tempCacheDirMap,
+                metadataManager.getDirectoryTable(),
+                prefixKeyInDB, seekDirInDB, prefixPath, startKey, volumeName,
+                bucketName, countEntries, numEntries, deletedKeySet);
+          } finally {
+            metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+                bucketName);
+          }
 
-            // Order of seek -> (1) Seek dirs only in dirTable. In OM, always
-            // the order of search is, first seek into fileTable and then
-            // dirTable. So, its not required to search again in the fileTable.
+          // Now first add all entries in the dir cache, if numEntries required

Review comment:
       Done

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2318,110 +2319,111 @@ private void listStatusFindKeyInTableCache(
       startKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
     }
 
+    // Note: eliminating the case where startCacheKey could end with '//'
+    String keyArgs = OzoneFSUtils.addTrailingSlashIfNeeded(
+        metadataManager.getOzoneKey(volumeName, bucketName, keyName));
+
     metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
         bucketName);
+
+    Table keyTable = metadataManager.getKeyTable();
     try {
-      Table keyTable = metadataManager.getKeyTable();
-      Iterator<Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>>>
+      Iterator< Map.Entry< CacheKey< String >, CacheValue< OmKeyInfo > > >
           cacheIter = keyTable.cacheIterator();
       String startCacheKey = OZONE_URI_DELIMITER + volumeName +
           OZONE_URI_DELIMITER + bucketName + OZONE_URI_DELIMITER +
           ((startKey.equals(OZONE_URI_DELIMITER)) ? "" : startKey);
-      // Note: eliminating the case where startCacheKey could end with '//'
-      String keyArgs = OzoneFSUtils.addTrailingSlashIfNeeded(
-          metadataManager.getOzoneKey(volumeName, bucketName, keyName));
 
       // First, find key in TableCache
       listStatusFindKeyInTableCache(cacheIter, keyArgs, startCacheKey,
           recursive, cacheKeyMap, deletedKeySet);
-      // Then, find key in DB
-      String seekKeyInDb =
-          metadataManager.getOzoneKey(volumeName, bucketName, startKey);
-      TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
-          iterator = keyTable.iterator();
-      iterator.seek(seekKeyInDb);
-      int countEntries = 0;
-      if (iterator.hasNext()) {
-        if (iterator.key().equals(keyArgs)) {
-          // Skip the key itself, since we are listing inside the directory
-          iterator.next();
-        }
-        // Iterate through seek results
-        while (iterator.hasNext() && numEntries - countEntries > 0) {
-          String entryInDb = iterator.key();
-          OmKeyInfo omKeyInfo = iterator.value().getValue();
-          if (entryInDb.startsWith(keyArgs)) {
-            String entryKeyName = omKeyInfo.getKeyName();
-            if (recursive) {
-              // for recursive list all the entries
+    } finally {
+      metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
+          bucketName);
+    }
+
+    // Then, find key in DB
+    String seekKeyInDb =
+        metadataManager.getOzoneKey(volumeName, bucketName, startKey);
+    TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+        iterator = keyTable.iterator();

Review comment:
       moved inside lock




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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