From 2511f13871a1f3869f5375639d4fcd81a303af1f Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Mon, 6 Jul 2020 11:37:42 +0530 Subject: [PATCH 1/6] HDDS-3824: OM read requests should make SCM#refreshPipeline outside the BUCKET_LOCK --- .../hadoop/ozone/om/KeyManagerImpl.java | 119 ++++++++++-------- 1 file changed, 65 insertions(+), 54 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 9e12e131899f..ad380095c187 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 @@ -638,10 +638,11 @@ public OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) String keyName = args.getKeyName(); metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, bucketName); + OmKeyInfo value = null; try { String keyBytes = metadataManager.getOzoneKey( volumeName, bucketName, keyName); - OmKeyInfo value = metadataManager.getKeyTable().get(keyBytes); + value = metadataManager.getKeyTable().get(keyBytes); if (value == null) { LOG.debug("volume:{} bucket:{} Key:{} not found", volumeName, bucketName, keyName); @@ -659,15 +660,6 @@ public OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) }); } } - // Refresh container pipeline info from SCM - // based on OmKeyArgs.refreshPipeline flag - if (args.getRefreshPipeline()) { - refreshPipeline(value); - } - if (args.getSortDatanodes()) { - sortDatanodeInPipeline(value, clientAddress); - } - return value; } catch (IOException ex) { if (ex instanceof OMException) { throw ex; @@ -680,6 +672,16 @@ public OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, bucketName); } + // Refresh container pipeline info from SCM + // based on OmKeyArgs.refreshPipeline flag + // value won't be null as the check is done inside try/catch block. + if (args.getRefreshPipeline()) { + refreshPipeline(value); + } + if (args.getSortDatanodes()) { + sortDatanodeInPipeline(value, clientAddress); + } + return value; } /** @@ -1695,6 +1697,16 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { String bucketName = args.getBucketName(); String keyName = args.getKeyName(); + return getOzoneFileStatus(volumeName, bucketName, keyName, + args.getRefreshPipeline()); + } + + private OzoneFileStatus getOzoneFileStatus(String volumeName, + String bucketName, + String keyName, + boolean refreshPipeline) + throws IOException { + OmKeyInfo fileKeyInfo = null; metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, bucketName); try { @@ -1706,36 +1718,41 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { // Check if the key is a file. String fileKeyBytes = metadataManager.getOzoneKey( - volumeName, bucketName, keyName); - OmKeyInfo fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes); + volumeName, bucketName, keyName); + fileKeyInfo = metadataManager.getKeyTable().get(fileKeyBytes); + + // Check if the key is a directory. + if (fileKeyInfo == null) { + String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName); + String dirKeyBytes = metadataManager.getOzoneKey( + volumeName, bucketName, dirKey); + OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes); + if (dirKeyInfo != null) { + return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true); + } + } + } finally { + metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, + bucketName); + + // if the key is a file then do refresh pipeline info in OM by asking SCM if (fileKeyInfo != null) { - if (args.getRefreshPipeline()) { + if (refreshPipeline) { refreshPipeline(fileKeyInfo); } - // this is a file return new OzoneFileStatus(fileKeyInfo, scmBlockSize, false); } + } - String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName); - String dirKeyBytes = metadataManager.getOzoneKey( - volumeName, bucketName, dirKey); - OmKeyInfo dirKeyInfo = metadataManager.getKeyTable().get(dirKeyBytes); - if (dirKeyInfo != null) { - return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true); - } - - if (LOG.isDebugEnabled()) { - LOG.debug("Unable to get file status for the key: volume: {}, bucket:" + - " {}, key: {}, with error: No such file exists.", volumeName, - bucketName, keyName); - } - throw new OMException("Unable to get file status: volume: " + - volumeName + " bucket: " + bucketName + " key: " + keyName, - FILE_NOT_FOUND); - } finally { - metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, - bucketName); + // Key is not found, throws exception + if (LOG.isDebugEnabled()) { + LOG.debug("Unable to get file status for the key: volume: {}, bucket:" + + " {}, key: {}, with error: No such file exists.", volumeName, + bucketName, keyName); } + throw new OMException("Unable to get file status: volume: " + + volumeName + " bucket: " + bucketName + " key: " + keyName, + FILE_NOT_FOUND); } /** @@ -1877,26 +1894,18 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress) String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - - metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, - bucketName); - try { - OzoneFileStatus fileStatus = getFileStatus(args); - if (fileStatus.isFile()) { - if (args.getRefreshPipeline()) { - refreshPipeline(fileStatus.getKeyInfo()); - } - if (args.getSortDatanodes()) { - sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress); - } - return fileStatus.getKeyInfo(); - } + OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName, + keyName,false); //if key is not of type file or if key is not found we throw an exception - } finally { - metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, - bucketName); + if (fileStatus != null && fileStatus.isFile()) { + if (args.getRefreshPipeline()) { + refreshPipeline(fileStatus.getKeyInfo()); + } + if (args.getSortDatanodes()) { + sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress); + } + return fileStatus.getKeyInfo(); } - throw new OMException("Can not write to directory: " + keyName, ResultCodes.NOT_A_FILE); } @@ -2067,9 +2076,6 @@ public List listStatus(OmKeyArgs args, boolean recursive, for (Map.Entry entry : cacheKeyMap.entrySet()) { // No need to check if a key is deleted or not here, this is handled // when adding entries to cacheKeyMap from DB. - if (args.getRefreshPipeline()) { - refreshPipeline(entry.getValue().getKeyInfo()); - } fileStatusList.add(entry.getValue()); countEntries++; if (countEntries >= numEntries) { @@ -2083,6 +2089,11 @@ public List listStatus(OmKeyArgs args, boolean recursive, metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, bucketName); } + if (args.getRefreshPipeline()) { + for(OzoneFileStatus fileStatus : fileStatusList){ + refreshPipeline(fileStatus.getKeyInfo()); + } + } return fileStatusList; } From ea1206929d39031343fc19969f5a475c65f46f75 Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Tue, 7 Jul 2020 11:00:11 +0530 Subject: [PATCH 2/6] HDDS-3824: Fixed checkstyle warnings --- .../java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 ad380095c187..04867e9332f4 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 @@ -1747,8 +1747,8 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName, // Key is not found, throws exception if (LOG.isDebugEnabled()) { LOG.debug("Unable to get file status for the key: volume: {}, bucket:" + - " {}, key: {}, with error: No such file exists.", volumeName, - bucketName, keyName); + " {}, key: {}, with error: No such file exists.", + volumeName, bucketName, keyName); } throw new OMException("Unable to get file status: volume: " + volumeName + " bucket: " + bucketName + " key: " + keyName, @@ -1895,7 +1895,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress) String bucketName = args.getBucketName(); String keyName = args.getKeyName(); OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName, - keyName,false); + keyName, false); //if key is not of type file or if key is not found we throw an exception if (fileStatus != null && fileStatus.isFile()) { if (args.getRefreshPipeline()) { From 35471063461c8978378b19ac7123e9f9b848aa59 Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Wed, 8 Jul 2020 12:56:55 +0530 Subject: [PATCH 3/6] HDDS-3824: Fixed review comment - generating of the secret token can also be moved outside lock --- .../hadoop/ozone/om/KeyManagerImpl.java | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 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 04867e9332f4..d951cd6b1dc4 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 @@ -643,35 +643,38 @@ public OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) String keyBytes = metadataManager.getOzoneKey( volumeName, bucketName, keyName); value = metadataManager.getKeyTable().get(keyBytes); - if (value == null) { - LOG.debug("volume:{} bucket:{} Key:{} not found", - volumeName, bucketName, keyName); - throw new OMException("Key not found", - KEY_NOT_FOUND); - } - if (grpcBlockTokenEnabled) { - String remoteUser = getRemoteUser().getShortUserName(); - for (OmKeyLocationInfoGroup key : value.getKeyLocationVersions()) { - key.getLocationList().forEach(k -> { - k.setToken(secretManager.generateToken(remoteUser, - k.getBlockID().getContainerBlockID().toString(), - getAclForUser(remoteUser), - k.getLength())); - }); - } - } } catch (IOException ex) { if (ex instanceof OMException) { throw ex; } - LOG.debug("Get key failed for volume:{} bucket:{} key:{}", - volumeName, bucketName, keyName, ex); - throw new OMException(ex.getMessage(), - KEY_NOT_FOUND); + if (LOG.isDebugEnabled()) { + LOG.debug("Get key failed for volume:{} bucket:{} key:{}", volumeName, + bucketName, keyName, ex); + } + throw new OMException(ex.getMessage(), KEY_NOT_FOUND); } finally { metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName, bucketName); } + + if (value == null) { + if (LOG.isDebugEnabled()) { + LOG.debug("volume:{} bucket:{} Key:{} not found", volumeName, + bucketName, keyName); + } + throw new OMException("Key not found", KEY_NOT_FOUND); + } + if (grpcBlockTokenEnabled) { + String remoteUser = getRemoteUser().getShortUserName(); + for (OmKeyLocationInfoGroup key : value.getKeyLocationVersions()) { + key.getLocationList().forEach(k -> { + k.setToken(secretManager.generateToken(remoteUser, + k.getBlockID().getContainerBlockID().toString(), + getAclForUser(remoteUser), k.getLength())); + }); + } + } + // Refresh container pipeline info from SCM // based on OmKeyArgs.refreshPipeline flag // value won't be null as the check is done inside try/catch block. From c500b2a829b73dc85c6c867f0e56902606e0a352 Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Tue, 14 Jul 2020 10:33:34 +0530 Subject: [PATCH 4/6] HDDS-3824: Review comments --- .../hadoop/ozone/om/KeyManagerImpl.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 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 d951cd6b1dc4..97af7844cdc4 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 @@ -1701,13 +1701,15 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { String keyName = args.getKeyName(); return getOzoneFileStatus(volumeName, bucketName, keyName, - args.getRefreshPipeline()); + args.getRefreshPipeline(), false, null); } private OzoneFileStatus getOzoneFileStatus(String volumeName, String bucketName, String keyName, - boolean refreshPipeline) + boolean refreshPipeline, + boolean sortDatanodes, + String clientAddress) throws IOException { OmKeyInfo fileKeyInfo = null; metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName, @@ -1743,6 +1745,9 @@ private OzoneFileStatus getOzoneFileStatus(String volumeName, if (refreshPipeline) { refreshPipeline(fileKeyInfo); } + if (sortDatanodes) { + sortDatanodeInPipeline(fileKeyInfo, clientAddress); + } return new OzoneFileStatus(fileKeyInfo, scmBlockSize, false); } } @@ -1898,15 +1903,10 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress) String bucketName = args.getBucketName(); String keyName = args.getKeyName(); OzoneFileStatus fileStatus = getOzoneFileStatus(volumeName, bucketName, - keyName, false); + keyName, args.getRefreshPipeline(), args.getSortDatanodes(), + clientAddress); //if key is not of type file or if key is not found we throw an exception - if (fileStatus != null && fileStatus.isFile()) { - if (args.getRefreshPipeline()) { - refreshPipeline(fileStatus.getKeyInfo()); - } - if (args.getSortDatanodes()) { - sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress); - } + if (fileStatus.isFile()) { return fileStatus.getKeyInfo(); } throw new OMException("Can not write to directory: " + keyName, From fc330f26f0e43e97c9ea7a1a20c0e2e655787e6c Mon Sep 17 00:00:00 2001 From: Rakesh Radhakrishnan Date: Tue, 14 Jul 2020 12:17:15 +0530 Subject: [PATCH 5/6] trigger new CI check From 69a210011a6a62f856e1b81b5a8848685536fd7a Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 14 Jul 2020 11:17:43 +0200 Subject: [PATCH 6/6] trigger new CI check