diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java index aaac874c731d..2e632bf8eef8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java @@ -64,9 +64,8 @@ import org.apache.hadoop.hdds.utils.db.cache.CacheValue; -import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_ALREADY_EXISTS; -import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; import static org.apache.hadoop.ozone.om.request.file.OMFileRequest.OMDirectoryResult.DIRECTORY_EXISTS_IN_GIVENPATH; import static org.apache.hadoop.ozone.om.request.file.OMFileRequest.OMDirectoryResult.FILE_EXISTS_IN_GIVENPATH; import static org.apache.hadoop.ozone.om.request.file.OMFileRequest.OMDirectoryResult.NONE; @@ -142,16 +141,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); - // TODO: Not checking volume exist here, once we have full cache we can - // add volume exist check also. - - OmBucketInfo omBucketInfo = omMetadataManager.getBucketTable().get( - omMetadataManager.getBucketKey(volumeName, bucketName)); - - if (omBucketInfo == null) { - throw new OMException("Bucket not found " + bucketName, - BUCKET_NOT_FOUND); - } + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); // Need to check if any files exist in the given path, if they exist we // cannot create a directory with the given key. @@ -159,6 +149,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OMFileRequest.verifyFilesInPath(omMetadataManager, volumeName, bucketName, keyName, Paths.get(keyName)); + OmBucketInfo omBucketInfo = omMetadataManager.getBucketTable().get( + omMetadataManager.getBucketKey(volumeName, bucketName)); OmKeyInfo dirKeyInfo = null; if (omDirectoryResult == FILE_EXISTS || omDirectoryResult == FILE_EXISTS_IN_GIVENPATH) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java index 52af0a30d850..dae18c55e872 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java @@ -186,14 +186,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); - OmBucketInfo bucketInfo = - omMetadataManager.getBucketTable().get( - omMetadataManager.getBucketKey(volumeName, bucketName)); - - if (bucketInfo == null) { - throw new OMException("Bucket " + bucketName + " not found", - OMException.ResultCodes.BUCKET_NOT_FOUND); - } + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); if (keyName.length() == 0) { // Check if this is the root of the filesystem. @@ -258,6 +251,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } // do open key + OmBucketInfo bucketInfo = omMetadataManager.getBucketTable().get( + omMetadataManager.getBucketKey(volumeName, bucketName)); encryptionInfo = getFileEncryptionInfo(ozoneManager, bucketInfo); omKeyInfo = prepareKeyInfo(omMetadataManager, keyArgs, omMetadataManager.getOzoneKey(volumeName, bucketName, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index 28dfaa5c2d55..91d28934cf31 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -49,8 +49,7 @@ import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; -import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes - .KEY_NOT_FOUND; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; /** @@ -120,12 +119,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); - // Not doing bucket/volume checks here. In this way we can avoid db - // checks for them. - // TODO: Once we have volume/bucket full cache, we can add - // them back, as these checks will be inexpensive at that time. - OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey); + // Validate bucket and volume exists or not. + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); + OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(objectKey); if (omKeyInfo == null) { throw new OMException("Key not found", KEY_NOT_FOUND); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java index 6f7ff6052809..15c41d2ead7f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java @@ -129,10 +129,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); - // Not doing bucket/volume checks here. In this way we can avoid db - // checks for them. - // TODO: Once we have volume/bucket full cache, we can add - // them back, as these checks will be inexpensive at that time. + // Validate bucket and volume exists or not. + validateBucketAndVolume(omMetadataManager, volumeName, bucketName); // fromKeyName should exist String fromKey = omMetadataManager.getOzoneKey( diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMDirectoryCreateRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMDirectoryCreateRequest.java index 4e93b13ad498..ba9253dc97fe 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMDirectoryCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMDirectoryCreateRequest.java @@ -53,6 +53,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.VOLUME_NOT_FOUND; /** * Test OM directory create request. @@ -152,6 +153,37 @@ public void testValidateAndUpdateCache() throws Exception { } + @Test + public void testValidateAndUpdateCacheWithVolumeNotFound() throws Exception { + String volumeName = "vol1"; + String bucketName = "bucket1"; + String keyName = RandomStringUtils.randomAlphabetic(5); + for (int i =0; i< 3; i++) { + keyName += "/" + RandomStringUtils.randomAlphabetic(5); + } + + OMRequest omRequest = createDirectoryRequest(volumeName, bucketName, + keyName); + OMDirectoryCreateRequest omDirectoryCreateRequest = + new OMDirectoryCreateRequest(omRequest); + + OMRequest modifiedOmRequest = + omDirectoryCreateRequest.preExecute(ozoneManager); + + omDirectoryCreateRequest = new OMDirectoryCreateRequest(modifiedOmRequest); + + OMClientResponse omClientResponse = + omDirectoryCreateRequest.validateAndUpdateCache(ozoneManager, 100L, + ozoneManagerDoubleBufferHelper); + + Assert.assertEquals(VOLUME_NOT_FOUND, + omClientResponse.getOMResponse().getStatus()); + + // Key should not exist in DB + Assert.assertNull(omMetadataManager.getKeyTable(). + get(omMetadataManager.getOzoneDirKey(volumeName, bucketName, keyName))); + + } @Test public void testValidateAndUpdateCacheWithBucketNotFound() throws Exception { @@ -171,6 +203,7 @@ public void testValidateAndUpdateCacheWithBucketNotFound() throws Exception { omDirectoryCreateRequest.preExecute(ozoneManager); omDirectoryCreateRequest = new OMDirectoryCreateRequest(modifiedOmRequest); + TestOMRequestUtils.addVolumeToDB(volumeName, omMetadataManager); OMClientResponse omClientResponse = omDirectoryCreateRequest.validateAndUpdateCache(ozoneManager, 100L, @@ -183,7 +216,6 @@ public void testValidateAndUpdateCacheWithBucketNotFound() throws Exception { Assert.assertTrue(omMetadataManager.getKeyTable().get( omMetadataManager.getOzoneDirKey( volumeName, bucketName, keyName)) == null); - } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java index 9639af03e595..93527a19ab0c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMFileCreateRequest.java @@ -38,6 +38,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .OMRequest; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.VOLUME_NOT_FOUND; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.FILE_ALREADY_EXISTS; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.NOT_A_FILE; @@ -177,6 +178,26 @@ public void testValidateAndUpdateCache() throws Exception { } + @Test + public void testValidateAndUpdateCacheWithVolumeNotFound() throws Exception { + OMRequest omRequest = createFileRequest(volumeName, bucketName, keyName, + HddsProtos.ReplicationFactor.ONE, HddsProtos.ReplicationType.RATIS, + false, true); + + OMFileCreateRequest omFileCreateRequest = new OMFileCreateRequest( + omRequest); + + OMRequest modifiedOmRequest = omFileCreateRequest.preExecute(ozoneManager); + + omFileCreateRequest = new OMFileCreateRequest(modifiedOmRequest); + + OMClientResponse omFileCreateResponse = + omFileCreateRequest.validateAndUpdateCache(ozoneManager, 100L, + ozoneManagerDoubleBufferHelper); + Assert.assertEquals(VOLUME_NOT_FOUND, + omFileCreateResponse.getOMResponse().getStatus()); + + } @Test public void testValidateAndUpdateCacheWithBucketNotFound() throws Exception { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java index e95ecd54396e..94e810dd90e2 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequest.java @@ -107,26 +107,40 @@ public void testValidateAndUpdateCacheWithKeyNotFound() throws Exception { omClientResponse.getOMResponse().getStatus()); } + @Test - public void testValidateAndUpdateCacheWithOutVolumeAndBucket() - throws Exception { + public void testValidateAndUpdateCacheWithVolumeNotFound() throws Exception { OMRequest modifiedOmRequest = doPreExecute(createDeleteKeyRequest()); OMKeyDeleteRequest omKeyDeleteRequest = new OMKeyDeleteRequest(modifiedOmRequest); - // In actual implementation we don't check for bucket/volume exists - // during delete key. So it should still return error KEY_NOT_FOUND - OMClientResponse omClientResponse = omKeyDeleteRequest.validateAndUpdateCache(ozoneManager, 100L, ozoneManagerDoubleBufferHelper); - Assert.assertEquals(OzoneManagerProtocolProtos.Status.KEY_NOT_FOUND, + Assert.assertEquals(OzoneManagerProtocolProtos.Status.VOLUME_NOT_FOUND, omClientResponse.getOMResponse().getStatus()); } + @Test + public void testValidateAndUpdateCacheWithBucketNotFound() throws Exception { + OMRequest modifiedOmRequest = + doPreExecute(createDeleteKeyRequest()); + + OMKeyDeleteRequest omKeyDeleteRequest = + new OMKeyDeleteRequest(modifiedOmRequest); + + TestOMRequestUtils.addVolumeToDB(volumeName, omMetadataManager); + + OMClientResponse omClientResponse = + omKeyDeleteRequest.validateAndUpdateCache(ozoneManager, + 100L, ozoneManagerDoubleBufferHelper); + + Assert.assertEquals(OzoneManagerProtocolProtos.Status.BUCKET_NOT_FOUND, + omClientResponse.getOMResponse().getStatus()); + } /** * This method calls preExecute and verify the modified request. diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequest.java index 864ba06111ac..57b1c9cc5b46 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRenameRequest.java @@ -49,6 +49,8 @@ public void testValidateAndUpdateCache() throws Exception { OMRequest modifiedOmRequest = doPreExecute(createRenameKeyRequest(toKeyName)); + TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); TestOMRequestUtils.addKeyToTable(false, volumeName, bucketName, keyName, clientID, replicationType, replicationFactor, omMetadataManager); @@ -112,16 +114,32 @@ public void testValidateAndUpdateCacheWithKeyNotFound() throws Exception { } + @Test + public void testValidateAndUpdateCacheWithVolumeNotFound() throws Exception { + String toKeyName = UUID.randomUUID().toString(); + OMRequest modifiedOmRequest = + doPreExecute(createRenameKeyRequest(toKeyName)); + + OMKeyRenameRequest omKeyRenameRequest = + new OMKeyRenameRequest(modifiedOmRequest); + + OMClientResponse omKeyRenameResponse = + omKeyRenameRequest.validateAndUpdateCache(ozoneManager, 100L, + ozoneManagerDoubleBufferHelper); + + Assert.assertEquals(OzoneManagerProtocolProtos.Status.VOLUME_NOT_FOUND, + omKeyRenameResponse.getOMResponse().getStatus()); + + } @Test - public void testValidateAndUpdateCacheWithOutVolumeAndBucket() - throws Exception { + public void testValidateAndUpdateCacheWithBucketNotFound() throws Exception { String toKeyName = UUID.randomUUID().toString(); OMRequest modifiedOmRequest = doPreExecute(createRenameKeyRequest(toKeyName)); - // In actual implementation we don't check for bucket/volume exists - // during delete key. So it should still return error KEY_NOT_FOUND + // Add only volume entry to DB. + TestOMRequestUtils.addVolumeToDB(volumeName, omMetadataManager); OMKeyRenameRequest omKeyRenameRequest = new OMKeyRenameRequest(modifiedOmRequest); @@ -130,7 +148,7 @@ public void testValidateAndUpdateCacheWithOutVolumeAndBucket() omKeyRenameRequest.validateAndUpdateCache(ozoneManager, 100L, ozoneManagerDoubleBufferHelper); - Assert.assertEquals(OzoneManagerProtocolProtos.Status.KEY_NOT_FOUND, + Assert.assertEquals(OzoneManagerProtocolProtos.Status.BUCKET_NOT_FOUND, omKeyRenameResponse.getOMResponse().getStatus()); }