You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ad...@apache.org on 2022/03/03 13:13:05 UTC
[ozone] branch master updated: HDDS-6321. Avoid refresh pipeline for key lookup in checkAcls (#3103)
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new e716c18 HDDS-6321. Avoid refresh pipeline for key lookup in checkAcls (#3103)
e716c18 is described below
commit e716c1810e065ce320c6d76c3d369c6090d8cc51
Author: Doroszlai, Attila <64...@users.noreply.github.com>
AuthorDate: Thu Mar 3 14:12:38 2022 +0100
HDDS-6321. Avoid refresh pipeline for key lookup in checkAcls (#3103)
---
.../apache/hadoop/ozone/om/TestKeyManagerImpl.java | 22 ++++-
.../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 103 +++++++++++----------
.../hadoop/ozone/om/OzonePrefixPathImpl.java | 2 +
.../hadoop/ozone/om/TrashOzoneFileSystem.java | 1 +
.../protocolPB/OzoneManagerRequestHandler.java | 1 +
5 files changed, 75 insertions(+), 54 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 68cc824..0240374 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
@@ -122,10 +122,14 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.Timeout;
-import static org.mockito.Matchers.anyList;
+
import org.mockito.Mockito;
+
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -209,9 +213,9 @@ public class TestKeyManagerImpl {
Mockito.when(mockScmBlockLocationProtocol
.allocateBlock(Mockito.anyLong(), Mockito.anyInt(),
- Mockito.any(ReplicationConfig.class),
+ any(ReplicationConfig.class),
Mockito.anyString(),
- Mockito.any(ExcludeList.class))).thenThrow(
+ any(ExcludeList.class))).thenThrow(
new SCMException("SafeModePrecheck failed for allocateBlock",
ResultCodes.SAFE_MODE_EXCEPTION));
createVolume(VOLUME_NAME);
@@ -458,6 +462,7 @@ public class TestKeyManagerImpl {
@Test
public void testCheckAccessForFileKey() throws Exception {
+ // GIVEN
OmKeyArgs keyArgs = createBuilder()
.setKeyName("testdir/deep/NOTICE.txt")
.build();
@@ -466,12 +471,19 @@ public class TestKeyManagerImpl {
keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
writeClient.commitKey(keyArgs, keySession.getId());
+ reset(mockScmContainerClient);
OzoneObj fileKey = OzoneObjInfo.Builder.fromKeyArgs(keyArgs)
.setStoreType(OzoneObj.StoreType.OZONE)
.build();
RequestContext context = currentUserReads();
- Assert.assertTrue(keyManager.checkAccess(fileKey, context));
+ // WHEN
+ boolean access = keyManager.checkAccess(fileKey, context);
+
+ // THEN
+ Assert.assertTrue(access);
+ verify(mockScmContainerClient, never())
+ .getContainerWithPipelineBatch(any());
}
@Test
@@ -1338,7 +1350,7 @@ public class TestKeyManagerImpl {
StorageContainerLocationProtocol sclProtocolMock = mock(
StorageContainerLocationProtocol.class);
doThrow(new IOException(errorMessage)).when(sclProtocolMock)
- .getContainerWithPipelineBatch(anyList());
+ .getContainerWithPipelineBatch(any());
ScmClient scmClientMock = mock(ScmClient.class);
when(scmClientMock.getContainerClient()).thenReturn(sclProtocolMock);
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 8c3f295..58e0f33 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
@@ -1005,6 +1005,7 @@ public class KeyManagerImpl implements KeyManager {
.setVolumeName(volume)
.setBucketName(bucket)
.setKeyName(keyName)
+ .setHeadOp(true)
.build();
BucketLayout bucketLayout = BucketLayout.DEFAULT;
@@ -1189,26 +1190,21 @@ public class KeyManagerImpl implements KeyManager {
Preconditions.checkNotNull(args, "Key args can not be null");
String volumeName = args.getVolumeName();
String bucketName = args.getBucketName();
- String keyName = args.getKeyName();
if (isBucketFSOptimized(volumeName, bucketName)) {
- return getOzoneFileStatusFSO(volumeName, bucketName, keyName,
- args.getSortDatanodes(), clientAddress,
- args.getLatestVersionLocation(), false);
- }
- return getOzoneFileStatus(volumeName, bucketName, keyName,
- args.getRefreshPipeline(), args.getSortDatanodes(),
- args.getLatestVersionLocation(), clientAddress);
- }
-
- private OzoneFileStatus getOzoneFileStatus(String volumeName,
- String bucketName,
- String keyName,
- boolean refreshPipeline,
- boolean sortDatanodes,
- boolean latestLocationVersion,
- String clientAddress)
- throws IOException {
+ return getOzoneFileStatusFSO(args, clientAddress, false);
+ }
+ return getOzoneFileStatus(args, clientAddress);
+ }
+
+ private OzoneFileStatus getOzoneFileStatus(OmKeyArgs args,
+ String clientAddress) throws IOException {
+
+ Preconditions.checkNotNull(args, "Key args can not be null");
+ final String volumeName = args.getVolumeName();
+ final String bucketName = args.getBucketName();
+ final String keyName = args.getKeyName();
+
OmKeyInfo fileKeyInfo = null;
metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
bucketName);
@@ -1244,15 +1240,19 @@ public class KeyManagerImpl implements KeyManager {
// if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
- if (latestLocationVersion) {
+ if (args.getLatestVersionLocation()) {
slimLocationVersion(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.
- refresh(fileKeyInfo);
- if (sortDatanodes) {
- sortDatanodes(clientAddress, fileKeyInfo);
+ // If operation is head, do not perform any additional steps
+ // As head operation does not need any of those details.
+ if (!args.isHeadOp()) {
+ // refreshPipeline flag check has been removed as part of
+ // https://issues.apache.org/jira/browse/HDDS-3658.
+ // Please refer this jira for more details.
+ refresh(fileKeyInfo);
+ if (args.getSortDatanodes()) {
+ sortDatanodes(clientAddress, fileKeyInfo);
+ }
}
return new OzoneFileStatus(fileKeyInfo, scmBlockSize, false);
}
@@ -1270,10 +1270,11 @@ public class KeyManagerImpl implements KeyManager {
}
- private OzoneFileStatus getOzoneFileStatusFSO(String volumeName,
- String bucketName, String keyName, boolean sortDatanodes,
- String clientAddress, boolean latestLocationVersion,
- boolean skipFileNotFoundError) throws IOException {
+ private OzoneFileStatus getOzoneFileStatusFSO(OmKeyArgs args,
+ String clientAddress, boolean skipFileNotFoundError) throws IOException {
+ final String volumeName = args.getVolumeName();
+ final String bucketName = args.getBucketName();
+ final String keyName = args.getKeyName();
OzoneFileStatus fileStatus = null;
metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
bucketName);
@@ -1296,16 +1297,19 @@ public class KeyManagerImpl implements KeyManager {
// if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileStatus.isFile()) {
OmKeyInfo fileKeyInfo = fileStatus.getKeyInfo();
- if (latestLocationVersion) {
+ if (args.getLatestVersionLocation()) {
slimLocationVersion(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.
- refresh(fileKeyInfo);
- if (sortDatanodes) {
- sortDatanodes(clientAddress, fileKeyInfo);
+ if (!args.isHeadOp()) {
+ // refreshPipeline flag check has been removed as part of
+ // https://issues.apache.org/jira/browse/HDDS-3658.
+ // Please refer this jira for more details.
+ refresh(fileKeyInfo);
+
+ if (args.getSortDatanodes()) {
+ sortDatanodes(clientAddress, fileKeyInfo);
+ }
}
return new OzoneFileStatus(fileKeyInfo, scmBlockSize, false);
} else {
@@ -1369,18 +1373,16 @@ public class KeyManagerImpl implements KeyManager {
String keyName = args.getKeyName();
OzoneFileStatus fileStatus;
if (isBucketFSOptimized(volumeName, bucketName)) {
- fileStatus = getOzoneFileStatusFSO(volumeName, bucketName, keyName,
- args.getSortDatanodes(), clientAddress,
- args.getLatestVersionLocation(), false);
+ fileStatus = getOzoneFileStatusFSO(args, clientAddress, false);
} else {
- fileStatus = getOzoneFileStatus(volumeName, bucketName,
- keyName, args.getRefreshPipeline(), args.getSortDatanodes(),
- args.getLatestVersionLocation(), clientAddress);
+ fileStatus = getOzoneFileStatus(args, clientAddress);
}
//if key is not of type file or if key is not found we throw an exception
if (fileStatus.isFile()) {
// add block token for read.
- addBlockToken4Read(fileStatus.getKeyInfo());
+ if (!args.isHeadOp()) {
+ addBlockToken4Read(fileStatus.getKeyInfo());
+ }
return fileStatus.getKeyInfo();
}
throw new OMException("Can not write to directory: " + keyName,
@@ -1655,9 +1657,9 @@ public class KeyManagerImpl implements KeyManager {
TreeMap<String, OzoneFileStatus> cacheFileMap = new TreeMap<>();
TreeMap<String, OzoneFileStatus> cacheDirMap = new TreeMap<>();
- String volumeName = args.getVolumeName();
- String bucketName = args.getBucketName();
- String keyName = args.getKeyName();
+ final String volumeName = args.getVolumeName();
+ final String bucketName = args.getBucketName();
+ final String keyName = args.getKeyName();
String seekFileInDB;
String seekDirInDB;
long prefixKeyInDB;
@@ -1758,9 +1760,12 @@ public class KeyManagerImpl implements KeyManager {
prefixPath = OzoneFSUtils.getParentDir(startKey);
}
- OzoneFileStatus fileStatusInfo = getOzoneFileStatusFSO(volumeName,
- bucketName, startKey, false, null,
- args.getLatestVersionLocation(), true);
+ OmKeyArgs startKeyArgs = args.toBuilder()
+ .setKeyName(startKey)
+ .setSortDatanodesInPipeline(false)
+ .build();
+ OzoneFileStatus fileStatusInfo = getOzoneFileStatusFSO(startKeyArgs,
+ null, true);
if (fileStatusInfo != null) {
prefixKeyInDB = fileStatusInfo.getKeyInfo().getParentObjectID();
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java
index b325952..2869d13 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java
@@ -57,6 +57,7 @@ public class OzonePrefixPathImpl implements OzonePrefixPath {
.setBucketName(bucketName)
.setKeyName(keyPrefix)
.setRefreshPipeline(false)
+ .setHeadOp(true)
.build();
try {
pathStatus = keyManager.getFileStatus(omKeyArgs);
@@ -158,6 +159,7 @@ public class OzonePrefixPathImpl implements OzonePrefixPath {
.setBucketName(bucketName)
.setKeyName(keyPrefix)
.setRefreshPipeline(false)
+ .setHeadOp(true)
.build();
List<OzoneFileStatus> statuses = keyManager.listStatus(omKeyArgs, false,
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java
index 68d3814..b27ae73 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashOzoneFileSystem.java
@@ -287,6 +287,7 @@ public class TrashOzoneFileSystem extends FileSystem {
.setVolumeName(volume)
.setBucketName(bucket)
.setKeyName(key)
+ .setHeadOp(true)
.build();
return keyArgs;
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
index dbc63e3..fd98078 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java
@@ -589,6 +589,7 @@ public class OzoneManagerRequestHandler implements RequestHandler {
.setKeyName(keyArgs.getKeyName())
.setRefreshPipeline(true)
.setLatestVersionLocation(keyArgs.getLatestVersionLocation())
+ .setHeadOp(keyArgs.getHeadOp())
.build();
List<OzoneFileStatus> statuses =
impl.listStatus(omKeyArgs, request.getRecursive(),
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org