From 98663c83f9f87815b92d24faf41f6041fae71e68 Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Thu, 3 Sep 2020 16:49:57 +0530 Subject: [PATCH 1/5] HDDS-3947: Sort DNs for client when the key is a file for #getFileStatus #listStatus APIs --- .../hadoop/ozone/client/rpc/RpcClient.java | 2 ++ ...ManagerProtocolClientSideTranslatorPB.java | 2 ++ .../hadoop/ozone/om/KeyManagerImpl.java | 36 ++++++++++++++++--- .../apache/hadoop/ozone/om/OzoneManager.java | 5 +-- .../hadoop/ozone/om/fs/OzoneManagerFS.java | 35 +++++++++++++++++- 5 files changed, 72 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index 3f984d279161..d72d930e54f3 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -1030,6 +1030,7 @@ public OzoneFileStatus getOzoneFileStatus(String volumeName, .setBucketName(bucketName) .setKeyName(keyName) .setRefreshPipeline(true) + .setSortDatanodesInPipeline(topologyAwareReadEnabled) .build(); return ozoneManagerClient.getFileStatus(keyArgs); } @@ -1112,6 +1113,7 @@ public List listStatus(String volumeName, String bucketName, .setBucketName(bucketName) .setKeyName(keyName) .setRefreshPipeline(true) + .setSortDatanodesInPipeline(topologyAwareReadEnabled) .build(); return ozoneManagerClient .listStatus(keyArgs, recursive, startKey, numEntries); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java index a6ea0423bcf0..506d84cbca19 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java @@ -1177,6 +1177,7 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { .setVolumeName(args.getVolumeName()) .setBucketName(args.getBucketName()) .setKeyName(args.getKeyName()) + .setSortDatanodes(args.getSortDatanodes()) .build(); GetFileStatusRequest req = GetFileStatusRequest.newBuilder() @@ -1390,6 +1391,7 @@ public List listStatus(OmKeyArgs args, boolean recursive, .setVolumeName(args.getVolumeName()) .setBucketName(args.getBucketName()) .setKeyName(args.getKeyName()) + .setSortDatanodes(args.getSortDatanodes()) .build(); ListStatusRequest listStatusRequest = ListStatusRequest.newBuilder() 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 28e091df497f..4d05f48ee0aa 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 @@ -1732,17 +1732,36 @@ private void validateOzoneObj(OzoneObj obj) throws OMException { * @param args Key args * @throws OMException if file does not exist * if bucket does not exist + * if volume does not exist * @throws IOException if there is error in the db * invalid arguments */ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { Preconditions.checkNotNull(args, "Key args can not be null"); + return getFileStatus(args, null); + } + + /** + * OzoneFS api to get file status for an entry. + * + * @param args Key args + * @param clientAddress a hint to key manager, order the datanode in returned + * pipeline by distance between client and datanode. + * @throws OMException if file does not exist + * if bucket does not exist + * if volume does not exist + * @throws IOException if there is error in the db + * invalid arguments + */ + public OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress) + throws IOException { + Preconditions.checkNotNull(args, "Key args can not be null"); String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); return getOzoneFileStatus(volumeName, bucketName, keyName, - args.getRefreshPipeline(), false, null); + args.getRefreshPipeline(), args.getSortDatanodes(), clientAddress); } private OzoneFileStatus getOzoneFileStatus(String volumeName, @@ -1783,7 +1802,9 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName, // if the key is a file then do refresh pipeline info in OM by asking SCM if (fileKeyInfo != null) { - refreshPipeline(fileKeyInfo); + if (refreshPipeline) { + refreshPipeline(fileKeyInfo); + } if (sortDatanodes) { sortDatanodeInPipeline(fileKeyInfo, clientAddress); } @@ -2011,7 +2032,8 @@ private void listStatusFindKeyInTableCache( * @return list of file status */ public List listStatus(OmKeyArgs args, boolean recursive, - String startKey, long numEntries) throws IOException { + String startKey, long numEntries, String clientAddress) + throws IOException { Preconditions.checkNotNull(args, "Key args can not be null"); List fileStatusList = new ArrayList<>(); @@ -2144,10 +2166,14 @@ public List listStatus(OmKeyArgs args, boolean recursive, metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, bucketName); } - if (args.getRefreshPipeline()) { - for(OzoneFileStatus fileStatus : fileStatusList){ + + for (OzoneFileStatus fileStatus : fileStatusList) { + if (args.getRefreshPipeline()) { refreshPipeline(fileStatus.getKeyInfo()); } + if (args.getSortDatanodes()) { + sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress); + } } return fileStatusList; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 9daeec16fe46..e43524aee2a7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -2797,7 +2797,7 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { try { metrics.incNumGetFileStatus(); - return keyManager.getFileStatus(args); + return keyManager.getFileStatus(args, getClientAddress()); } catch (IOException ex) { metrics.incNumGetFileStatusFails(); auditSuccess = false; @@ -2921,7 +2921,8 @@ public List listStatus(OmKeyArgs args, boolean recursive, try { metrics.incNumListStatus(); - return keyManager.listStatus(args, recursive, startKey, numEntries); + return keyManager.listStatus(args, recursive, startKey, numEntries, + getClientAddress()); } catch (Exception ex) { metrics.incNumListStatusFails(); auditSuccess = false; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java index 647931af0d04..e8384a8e2f43 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java @@ -31,8 +31,27 @@ * Ozone Manager FileSystem interface. */ public interface OzoneManagerFS extends IOzoneAcl { + + /** + * Get file status for a file or a directory. + * + * @param args the args of the key provided by client. + * @return file status. + * @throws IOException if file or bucket or volume does not exist + */ OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException; + /** + * Get file status for a file or a directory. + * + * @param args the args of the key provided by client. + * @param clientAddress a hint to key manager, order the datanode in returned + * pipeline by distance between client and datanode. + * @return file status. + * @throws IOException if file or bucket or volume does not exist + */ + OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress) throws IOException; + void createDirectory(OmKeyArgs args) throws IOException; OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, @@ -49,6 +68,20 @@ OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, */ OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress) throws IOException; + /** + * List the status for a file or a directory and its contents. + * + * @param keyArgs the args of the key provided by client. + * @param recursive For a directory if true all the descendants of a + * particular directory are listed + * @param startKey Key from which listing needs to start. If startKey + * exists its status is included in the final list. + * @param numEntries Number of entries to list from the start key + * @param clientAddress a hint to key manager, order the datanode in returned + * pipeline by distance between client and datanode. + * @return list of file status + * @throws IOException if file or bucket or volume does not exist + */ List listStatus(OmKeyArgs keyArgs, boolean recursive, - String startKey, long numEntries) throws IOException; + String startKey, long numEntries, String clientAddress) throws IOException; } From b016bfa01649b9d5d5cb2f98c92980b490c35a6e Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Thu, 3 Sep 2020 17:04:02 +0530 Subject: [PATCH 2/5] Fixed TestKeyManagerImpl conflicts --- .../hadoop/ozone/om/TestKeyManagerImpl.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java index a2b70a52f2f7..d0d58fc846f5 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java @@ -210,7 +210,8 @@ public static void cleanup() throws Exception { @After public void cleanupTest() throws IOException { List fileStatuses = keyManager - .listStatus(createBuilder().setKeyName("").build(), true, "", 100000); + .listStatus(createBuilder().setKeyName("").build(), true, "", 100000, + null); for (OzoneFileStatus fileStatus : fileStatuses) { if (fileStatus.isFile()) { keyManager.deleteKey( @@ -833,17 +834,17 @@ public void testListStatusWithTableCache() throws Exception { OmKeyArgs rootDirArgs = createKeyArgs(""); // Get entries in both TableCache and DB List fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000); + keyManager.listStatus(rootDirArgs, true, "", 1000, null); Assert.assertEquals(100, fileStatuses.size()); // Get entries with startKey=prefixKeyInDB fileStatuses = - keyManager.listStatus(rootDirArgs, true, prefixKeyInDB, 1000); + keyManager.listStatus(rootDirArgs, true, prefixKeyInDB, 1000, null); Assert.assertEquals(50, fileStatuses.size()); // Get entries with startKey=prefixKeyInCache fileStatuses = - keyManager.listStatus(rootDirArgs, true, prefixKeyInCache, 1000); + keyManager.listStatus(rootDirArgs, true, prefixKeyInCache, 1000, null); Assert.assertEquals(100, fileStatuses.size()); // Clean up cache by marking those keys in cache as deleted @@ -875,12 +876,12 @@ public void testListStatusWithTableCacheRecursive() throws Exception { OmKeyArgs rootDirArgs = createKeyArgs(""); // Test listStatus with recursive=false, should only have dirs under root List fileStatuses = - keyManager.listStatus(rootDirArgs, false, "", 1000); + keyManager.listStatus(rootDirArgs, false, "", 1000, null); Assert.assertEquals(2, fileStatuses.size()); // Test listStatus with recursive=true, should have dirs under root and fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000); + keyManager.listStatus(rootDirArgs, true, "", 1000, null); Assert.assertEquals(3, fileStatuses.size()); // Add a total of 10 key entries to DB and TableCache under dir1 @@ -904,12 +905,12 @@ public void testListStatusWithTableCacheRecursive() throws Exception { // Test non-recursive, should return the dir under root fileStatuses = - keyManager.listStatus(rootDirArgs, false, "", 1000); + keyManager.listStatus(rootDirArgs, false, "", 1000, null); Assert.assertEquals(2, fileStatuses.size()); // Test recursive, should return the dir and the keys in it fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000); + keyManager.listStatus(rootDirArgs, true, "", 1000, null); Assert.assertEquals(10 + 3, fileStatuses.size()); // Clean up @@ -954,12 +955,12 @@ public void testListStatusWithDeletedEntriesInCache() throws Exception { OmKeyArgs rootDirArgs = createKeyArgs(""); List fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000); + keyManager.listStatus(rootDirArgs, true, "", 1000, null); // Should only get entries that are not marked as deleted. Assert.assertEquals(50, fileStatuses.size()); // Test startKey fileStatuses = - keyManager.listStatus(rootDirArgs, true, prefixKey, 1000); + keyManager.listStatus(rootDirArgs, true, prefixKey, 1000, null); // Should only get entries that are not marked as deleted. Assert.assertEquals(50, fileStatuses.size()); // Verify result @@ -991,7 +992,7 @@ public void testListStatusWithDeletedEntriesInCache() throws Exception { existKeySet.removeAll(deletedKeySet); fileStatuses = keyManager.listStatus( - rootDirArgs, true, "", 1000); + rootDirArgs, true, "", 1000, null); // Should only get entries that are not marked as deleted. Assert.assertEquals(50 / 2, fileStatuses.size()); @@ -1010,7 +1011,7 @@ public void testListStatusWithDeletedEntriesInCache() throws Exception { expectedKeys.clear(); do { fileStatuses = keyManager.listStatus( - rootDirArgs, true, startKey, batchSize); + rootDirArgs, true, startKey, batchSize, null); // Note fileStatuses will never be empty since we are using the last // keyName as the startKey of next batch, // the startKey itself will show up in the next batch of results. @@ -1058,11 +1059,11 @@ public void testListStatus() throws IOException { OmKeyArgs rootDirArgs = createKeyArgs(""); List fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 100); + keyManager.listStatus(rootDirArgs, true, "", 100, null); // verify the number of status returned is same as number of entries Assert.assertEquals(numEntries, fileStatuses.size()); - fileStatuses = keyManager.listStatus(rootDirArgs, false, "", 100); + fileStatuses = keyManager.listStatus(rootDirArgs, false, "", 100, null); // the number of immediate children of root is 1 Assert.assertEquals(1, fileStatuses.size()); @@ -1070,19 +1071,19 @@ public void testListStatus() throws IOException { // return all the entries. String startKey = children.iterator().next(); fileStatuses = keyManager.listStatus(rootDirArgs, true, - startKey.substring(0, startKey.length() - 1), 100); + startKey.substring(0, startKey.length() - 1), 100, null); Assert.assertEquals(numEntries, fileStatuses.size()); for (String directory : directorySet) { // verify status list received for each directory with recursive flag set // to false OmKeyArgs dirArgs = createKeyArgs(directory); - fileStatuses = keyManager.listStatus(dirArgs, false, "", 100); + fileStatuses = keyManager.listStatus(dirArgs, false, "", 100, null); verifyFileStatus(directory, fileStatuses, directorySet, fileSet, false); // verify status list received for each directory with recursive flag set // to true - fileStatuses = keyManager.listStatus(dirArgs, true, "", 100); + fileStatuses = keyManager.listStatus(dirArgs, true, "", 100, null); verifyFileStatus(directory, fileStatuses, directorySet, fileSet, true); // verify list status call with using the startKey parameter and @@ -1096,7 +1097,7 @@ public void testListStatus() throws IOException { tempFileStatus != null ? tempFileStatus.get(tempFileStatus.size() - 1).getKeyInfo() .getKeyName() : null, - 2); + 2, null); tmpStatusSet.addAll(tempFileStatus); } while (tempFileStatus.size() == 2); verifyFileStatus(directory, new ArrayList<>(tmpStatusSet), directorySet, @@ -1114,7 +1115,7 @@ public void testListStatus() throws IOException { tempFileStatus.get(tempFileStatus.size() - 1).getKeyInfo() .getKeyName() : null, - 2); + 2, null); tmpStatusSet.addAll(tempFileStatus); } while (tempFileStatus.size() == 2); verifyFileStatus(directory, new ArrayList<>(tmpStatusSet), directorySet, From 0ec0ee3c192b491b1bb43091f59f86e313f18db2 Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Fri, 4 Sep 2020 10:53:33 +0530 Subject: [PATCH 3/5] Fixed checkstyle warnings --- .../java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java index e8384a8e2f43..be28c9601b72 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java @@ -50,7 +50,8 @@ public interface OzoneManagerFS extends IOzoneAcl { * @return file status. * @throws IOException if file or bucket or volume does not exist */ - OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress) throws IOException; + OzoneFileStatus getFileStatus(OmKeyArgs args, String clientAddress) + throws IOException; void createDirectory(OmKeyArgs args) throws IOException; @@ -83,5 +84,6 @@ OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, * @throws IOException if file or bucket or volume does not exist */ List listStatus(OmKeyArgs keyArgs, boolean recursive, - String startKey, long numEntries, String clientAddress) throws IOException; + String startKey, long numEntries, String clientAddress) + throws IOException; } From 088e40d2f79ab0e8c6021523fd4e2b8db23127af Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Wed, 16 Sep 2020 15:13:25 +0530 Subject: [PATCH 4/5] Fixed review comments - removed refreshPipeline check and added listStatus overloaded api --- .../hadoop/ozone/om/TestKeyManagerImpl.java | 42 +++++++++---------- .../hadoop/ozone/om/KeyManagerImpl.java | 26 ++++++++++-- .../hadoop/ozone/om/fs/OzoneManagerFS.java | 16 +++++++ 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java index d0d58fc846f5..e4fe449d3ab2 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java @@ -210,8 +210,7 @@ public static void cleanup() throws Exception { @After public void cleanupTest() throws IOException { List fileStatuses = keyManager - .listStatus(createBuilder().setKeyName("").build(), true, "", 100000, - null); + .listStatus(createBuilder().setKeyName("").build(), true, "", 100000); for (OzoneFileStatus fileStatus : fileStatuses) { if (fileStatus.isFile()) { keyManager.deleteKey( @@ -834,17 +833,17 @@ public void testListStatusWithTableCache() throws Exception { OmKeyArgs rootDirArgs = createKeyArgs(""); // Get entries in both TableCache and DB List fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000, null); + keyManager.listStatus(rootDirArgs, true, "", 1000); Assert.assertEquals(100, fileStatuses.size()); // Get entries with startKey=prefixKeyInDB fileStatuses = - keyManager.listStatus(rootDirArgs, true, prefixKeyInDB, 1000, null); + keyManager.listStatus(rootDirArgs, true, prefixKeyInDB, 1000); Assert.assertEquals(50, fileStatuses.size()); // Get entries with startKey=prefixKeyInCache fileStatuses = - keyManager.listStatus(rootDirArgs, true, prefixKeyInCache, 1000, null); + keyManager.listStatus(rootDirArgs, true, prefixKeyInCache, 1000); Assert.assertEquals(100, fileStatuses.size()); // Clean up cache by marking those keys in cache as deleted @@ -876,12 +875,12 @@ public void testListStatusWithTableCacheRecursive() throws Exception { OmKeyArgs rootDirArgs = createKeyArgs(""); // Test listStatus with recursive=false, should only have dirs under root List fileStatuses = - keyManager.listStatus(rootDirArgs, false, "", 1000, null); + keyManager.listStatus(rootDirArgs, false, "", 1000); Assert.assertEquals(2, fileStatuses.size()); // Test listStatus with recursive=true, should have dirs under root and fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000, null); + keyManager.listStatus(rootDirArgs, true, "", 1000); Assert.assertEquals(3, fileStatuses.size()); // Add a total of 10 key entries to DB and TableCache under dir1 @@ -905,12 +904,12 @@ public void testListStatusWithTableCacheRecursive() throws Exception { // Test non-recursive, should return the dir under root fileStatuses = - keyManager.listStatus(rootDirArgs, false, "", 1000, null); + keyManager.listStatus(rootDirArgs, false, "", 1000); Assert.assertEquals(2, fileStatuses.size()); // Test recursive, should return the dir and the keys in it fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000, null); + keyManager.listStatus(rootDirArgs, true, "", 1000); Assert.assertEquals(10 + 3, fileStatuses.size()); // Clean up @@ -955,12 +954,12 @@ public void testListStatusWithDeletedEntriesInCache() throws Exception { OmKeyArgs rootDirArgs = createKeyArgs(""); List fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 1000, null); + keyManager.listStatus(rootDirArgs, true, "", 1000); // Should only get entries that are not marked as deleted. Assert.assertEquals(50, fileStatuses.size()); // Test startKey fileStatuses = - keyManager.listStatus(rootDirArgs, true, prefixKey, 1000, null); + keyManager.listStatus(rootDirArgs, true, prefixKey, 1000); // Should only get entries that are not marked as deleted. Assert.assertEquals(50, fileStatuses.size()); // Verify result @@ -992,7 +991,7 @@ public void testListStatusWithDeletedEntriesInCache() throws Exception { existKeySet.removeAll(deletedKeySet); fileStatuses = keyManager.listStatus( - rootDirArgs, true, "", 1000, null); + rootDirArgs, true, "", 1000); // Should only get entries that are not marked as deleted. Assert.assertEquals(50 / 2, fileStatuses.size()); @@ -1011,7 +1010,7 @@ public void testListStatusWithDeletedEntriesInCache() throws Exception { expectedKeys.clear(); do { fileStatuses = keyManager.listStatus( - rootDirArgs, true, startKey, batchSize, null); + rootDirArgs, true, startKey, batchSize); // Note fileStatuses will never be empty since we are using the last // keyName as the startKey of next batch, // the startKey itself will show up in the next batch of results. @@ -1059,11 +1058,11 @@ public void testListStatus() throws IOException { OmKeyArgs rootDirArgs = createKeyArgs(""); List fileStatuses = - keyManager.listStatus(rootDirArgs, true, "", 100, null); + keyManager.listStatus(rootDirArgs, true, "", 100); // verify the number of status returned is same as number of entries Assert.assertEquals(numEntries, fileStatuses.size()); - fileStatuses = keyManager.listStatus(rootDirArgs, false, "", 100, null); + fileStatuses = keyManager.listStatus(rootDirArgs, false, "", 100); // the number of immediate children of root is 1 Assert.assertEquals(1, fileStatuses.size()); @@ -1071,19 +1070,19 @@ public void testListStatus() throws IOException { // return all the entries. String startKey = children.iterator().next(); fileStatuses = keyManager.listStatus(rootDirArgs, true, - startKey.substring(0, startKey.length() - 1), 100, null); + startKey.substring(0, startKey.length() - 1), 100); Assert.assertEquals(numEntries, fileStatuses.size()); for (String directory : directorySet) { // verify status list received for each directory with recursive flag set // to false OmKeyArgs dirArgs = createKeyArgs(directory); - fileStatuses = keyManager.listStatus(dirArgs, false, "", 100, null); + fileStatuses = keyManager.listStatus(dirArgs, false, "", 100); verifyFileStatus(directory, fileStatuses, directorySet, fileSet, false); // verify status list received for each directory with recursive flag set // to true - fileStatuses = keyManager.listStatus(dirArgs, true, "", 100, null); + fileStatuses = keyManager.listStatus(dirArgs, true, "", 100); verifyFileStatus(directory, fileStatuses, directorySet, fileSet, true); // verify list status call with using the startKey parameter and @@ -1096,8 +1095,7 @@ public void testListStatus() throws IOException { tempFileStatus = keyManager.listStatus(dirArgs, false, tempFileStatus != null ? tempFileStatus.get(tempFileStatus.size() - 1).getKeyInfo() - .getKeyName() : null, - 2, null); + .getKeyName() : null, 2); tmpStatusSet.addAll(tempFileStatus); } while (tempFileStatus.size() == 2); verifyFileStatus(directory, new ArrayList<>(tmpStatusSet), directorySet, @@ -1113,9 +1111,7 @@ public void testListStatus() throws IOException { tempFileStatus = keyManager.listStatus(dirArgs, true, tempFileStatus != null ? tempFileStatus.get(tempFileStatus.size() - 1).getKeyInfo() - .getKeyName() : - null, - 2, null); + .getKeyName() : null, 2); tmpStatusSet.addAll(tempFileStatus); } while (tempFileStatus.size() == 2); verifyFileStatus(directory, new ArrayList<>(tmpStatusSet), directorySet, 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 4d05f48ee0aa..681dc22caeab 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 @@ -1802,9 +1802,10 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName, // if the key is a file then do refresh pipeline info in OM by asking SCM if (fileKeyInfo != null) { - if (refreshPipeline) { - refreshPipeline(fileKeyInfo); - } + // 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(fileKeyInfo); if (sortDatanodes) { sortDatanodeInPipeline(fileKeyInfo, clientAddress); } @@ -2031,6 +2032,25 @@ private void listStatusFindKeyInTableCache( * @param numEntries Number of entries to list from the start key * @return list of file status */ + public List listStatus(OmKeyArgs args, boolean recursive, + String startKey, long numEntries) + throws IOException { + return listStatus(args, recursive, startKey, numEntries, null); + } + + /** + * List the status for a file or a directory and its contents. + * + * @param args Key args + * @param recursive For a directory if true all the descendants of a + * particular directory are listed + * @param startKey Key from which listing needs to start. If startKey exists + * its status is included in the final list. + * @param numEntries Number of entries to list from the start key + * @param clientAddress a hint to key manager, order the datanode in returned + * pipeline by distance between client and datanode. + * @return list of file status + */ public List listStatus(OmKeyArgs args, boolean recursive, String startKey, long numEntries, String clientAddress) throws IOException { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java index be28c9601b72..a7329c81ca3b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/fs/OzoneManagerFS.java @@ -69,6 +69,22 @@ OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, */ OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress) throws IOException; + /** + * List the status for a file or a directory and its contents. + * + * @param keyArgs the args of the key provided by client. + * @param recursive For a directory if true all the descendants of a + * particular directory are listed + * @param startKey Key from which listing needs to start. If startKey + * exists its status is included in the final list. + * @param numEntries Number of entries to list from the start key + * @return list of file status + * @throws IOException if file or bucket or volume does not exist + */ + List listStatus(OmKeyArgs keyArgs, boolean recursive, + String startKey, long numEntries) + throws IOException; + /** * List the status for a file or a directory and its contents. * From b48a1ba19a1c96c1e83842b59c0cb0ccff2b802c Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Thu, 17 Sep 2020 08:11:43 +0530 Subject: [PATCH 5/5] Addressed comment - added clientAddress to #getFileStatus call from #listStatus --- .../apache/hadoop/ozone/om/KeyManagerImpl.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) 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 681dc22caeab..14fc07e34864 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 @@ -2069,18 +2069,18 @@ public List listStatus(OmKeyArgs args, boolean recursive, // A set to keep track of keys deleted in cache but not flushed to DB. Set deletedKeySet = new TreeSet<>(); + if (Strings.isNullOrEmpty(startKey)) { + OzoneFileStatus fileStatus = getFileStatus(args, clientAddress); + if (fileStatus.isFile()) { + return Collections.singletonList(fileStatus); + } + // keyName is a directory + startKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName); + } + metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, bucketName); try { - if (Strings.isNullOrEmpty(startKey)) { - OzoneFileStatus fileStatus = getFileStatus(args); - if (fileStatus.isFile()) { - return Collections.singletonList(fileStatus); - } - // keyName is a directory - startKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName); - } - Table keyTable = metadataManager.getKeyTable(); Iterator, CacheValue>> cacheIter = keyTable.cacheIterator();