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 2022/03/11 14:27:47 UTC
[GitHub] [ozone] xBis7 opened a new pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
xBis7 opened a new pull request #3183:
URL: https://github.com/apache/ozone/pull/3183
## What changes were proposed in this pull request?
There has been implemented a new "write-path" for OzoneManager but many of the BucketManager tests still call the old "write-path" methods. In this PR the obsolete code for the methods performing writes in BucketManager and BucketManagerImpl classes was removed. Such methods include createBucket(), deleteBucket() and setBucketProperty().
There have also been changes in all the tests in TestBucketManagerImpl class that were using these methods. Instead of writing and deleting buckets using BucketManager class, now these actions are performed using OzoneManagerProtocol class.
Note that this is the same approach taken in the [PR which fixed the keymanager tests](https://github.com/apache/ozone/pull/2961).
Specifically, OmMetadataManager, BucketManager and MetadataManager classes are now accessed through OmTestManagers class, invalid volume and bucket names have been fixed and
`bucketManager.createBucket(bucketInfo);` has been replaced by
`writeClient.createBucket(bucketInfo);`,
`bucketManager.setBucketProperty(bucketArgs);` has been replaced by
`writeClient.setBucketProperty(bucketArgs);`,
`bucketManager.deleteBucket("sampleVol", "bucketOne");` has been replaced by
`writeClient.deleteBucket("sample-vol", "bucket-one");`
where `writeClient` is of type OzoneManagerProtocol.
Finally, in TestBucketManagerImpl class, the new writeClient approach allows us to fix the previously broken tests that were marked with `@Ignore`.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3370
## How was this patch tested?
All tests now passing, including the ones previously marked `@Ignore`
--
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
[GitHub] [ozone] GeorgeJahad commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol", "bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that call to fail with a VOLUME_NOT_FOUND exception. It was generating that exception because the call to create the volume updated the table without updating the cache. When getBucketInfo() sees its not in the cache, it throws the exception.
Now that the createSampleVol call correctly updates the cache, it actually breaks the test, so we removed it.
--
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
[GitHub] [ozone] hanishakoneru commented on pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#issuecomment-1075364407
Thanks @xBis7 for cleaning up the code and @adoroszlai for the review.
LGTM. +1. I will merge it.
--
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
[GitHub] [ozone] hanishakoneru merged pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
hanishakoneru merged pull request #3183:
URL: https://github.com/apache/ozone/pull/3183
--
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
[GitHub] [ozone] GeorgeJahad commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol", "bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that call to fail with a VOLUME_NOT_FOUND exception. It was generating that exception because the call to create the volume updated the table without updating the cache. When getBucketInfo() sees its not in the cache, it throws the exception.
Now that the create volume call correctly uses the cache, this createSampleVol() call actually breaks the test, so we removed it.
--
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
[GitHub] [ozone] GeorgeJahad commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol", "bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that call to fail with a VOLUME_NOT_FOUND exception. It was generating that exception because the call to create the volume updated the metaData manager without updating the cache. When getBucketInfo() sees its not in the cache, it throws the exception.
Now that the create volume call correctly uses the cache, this createSampleVol() call actually breaks the test, so we removed it.
--
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
[GitHub] [ozone] GeorgeJahad commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol", "bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that call to fail with a VOLUME_NOT_FOUND exception. It was generating that exception because the call to create the volume updated the table without updating the cache. When getBucketInfo() saw the volume was not in the cache, it would throw the exception.
Now that the createSampleVol call correctly updates the cache, it actually breaks the test, so we removed it.
--
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
[GitHub] [ozone] GeorgeJahad commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol", "bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that call to fail with a VOLUME_NOT_FOUND exception. It was generating that exception because the call to create the volume updated the table without updating the cache. When getBucketInfo() sees its not in the cache, it throws the exception.
Now that the createSampleVol call correctly uses the cache, it actually breaks the test, so we removed it.
--
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
[GitHub] [ozone] xBis7 commented on pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
xBis7 commented on pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#issuecomment-1075080182
@adoroszlai Thank you for your comments. I removed the methods and everything looks good.
--
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
[GitHub] [ozone] GeorgeJahad commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
GeorgeJahad commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r824993158
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java
##########
@@ -120,91 +131,86 @@ public void testCreateBucket() throws Exception {
Mockito.when(kmsProvider.getMetadata(testBekName)).thenReturn(mockMetadata);
Mockito.when(mockMetadata.getCipher()).thenReturn(testCipherName);
- BucketManager bucketManager = new BucketManagerImpl(metaMgr,
- kmsProvider);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.setBucketEncryptionKey(new
BucketEncryptionKeyInfo.Builder().setKeyName("key1").build())
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol", "bucketOne"));
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
OmBucketInfo bucketInfoRead =
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
Assert.assertTrue(bucketInfoRead.getEncryptionKeyInfo().getKeyName()
.equals(bucketInfo.getEncryptionKeyInfo().getKeyName()));
- metaMgr.getStore().close();
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateEncryptedBucket() throws Exception {
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
+ BucketManager bucketManager = omTestManagers.getBucketManager();
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- Assert.assertNotNull(bucketManager.getBucketInfo("sampleVol",
- "bucketOne"));
- metaMgr.getStore().close();
+ writeClient.createBucket(bucketInfo);
+ Assert.assertNotNull(bucketManager.getBucketInfo("sample-vol",
+ "bucket-one"));
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testCreateAlreadyExistingBucket() throws Exception {
thrown.expectMessage("Bucket already exist");
- OmMetadataManagerImpl metaMgr = createSampleVol();
+ createSampleVol();
try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
- .setVolumeName("sampleVol")
- .setBucketName("bucketOne")
+ .setVolumeName("sample-vol")
+ .setBucketName("bucket-one")
.build();
- bucketManager.createBucket(bucketInfo);
- bucketManager.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
+ writeClient.createBucket(bucketInfo);
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_ALREADY_EXISTS,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
- @Ignore("Bucket Manager does not use cache, Disable it for now.")
public void testGetBucketInfoForInvalidBucket() throws Exception {
thrown.expectMessage("Bucket not found");
- OmMetadataManagerImpl metaMgr = createSampleVol();
- try {
+ createSampleVol();
+ try {
- BucketManager bucketManager = new BucketManagerImpl(metaMgr);
- bucketManager.getBucketInfo("sampleVol", "bucketOne");
+ BucketManager bucketManager = omTestManagers.getBucketManager();
+ bucketManager.getBucketInfo("sample-vol", "bucket-one");
} catch (OMException omEx) {
Assert.assertEquals(ResultCodes.BUCKET_NOT_FOUND,
omEx.getResult());
throw omEx;
- } finally {
- metaMgr.getStore().close();
}
}
@Test
public void testGetBucketInfo() throws Exception {
- final String volumeName = "sampleVol";
- final String bucketName = "bucketOne";
- OmMetadataManagerImpl metaMgr = createSampleVol();
Review comment:
FYI: We believe that, prior to our changes, the testGetBucketInfo() test was actually incorrect, (even though it was passing.)
It would first create the volume, then do a getBucketInfo() expecting that call to fail with a VOLUME_NOT_FOUND exception. It was generating that exception because the call to create the volume updated the table without updating the cache. When getBucketInfo() saw the volume was not in the cache, it would throw the exception.
Now that the createSampleVol call correctly updates the cache, it actually breaks the test, so we removed it.
Now the getBucketInfo() call generates the VOLUME_NOT_FOUND error because the volume really isn't there.
--
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
[GitHub] [ozone] adoroszlai commented on pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#issuecomment-1075322461
Thanks @xBis7 for updating the patch. LGTM. Let's wait for @hanishakoneru to review, too.
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #3183: HDDS-3370. Cleanup old write-path of bucket in OM
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #3183:
URL: https://github.com/apache/ozone/pull/3183#discussion_r831531937
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java
##########
@@ -286,118 +195,6 @@ public OmBucketInfo getBucketInfo(String volumeName, String bucketName)
}
}
- /**
- * Sets bucket property from args.
- *
- * @param args - BucketArgs.
- * @throws IOException - On Failure.
- */
- @Override
- public void setBucketProperty(OmBucketArgs args) throws IOException {
- Preconditions.checkNotNull(args);
- String volumeName = args.getVolumeName();
- String bucketName = args.getBucketName();
- metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- String bucketKey = metadataManager.getBucketKey(volumeName, bucketName);
- OmBucketInfo oldBucketInfo =
- metadataManager.getBucketTable().get(bucketKey);
- //Check if bucket exist
- if (oldBucketInfo == null) {
- LOG.debug("bucket: {} not found ", bucketName);
- throw new OMException("Bucket doesn't exist",
- BUCKET_NOT_FOUND);
- }
- OmBucketInfo.Builder bucketInfoBuilder = OmBucketInfo.newBuilder();
- bucketInfoBuilder.setVolumeName(oldBucketInfo.getVolumeName())
- .setBucketName(oldBucketInfo.getBucketName());
- bucketInfoBuilder.addAllMetadata(args.getMetadata());
-
- //Check StorageType to update
- StorageType storageType = args.getStorageType();
- if (storageType != null) {
- bucketInfoBuilder.setStorageType(storageType);
- LOG.debug("Updating bucket storage type for bucket: {} in volume: {}",
- bucketName, volumeName);
- } else {
- bucketInfoBuilder.setStorageType(oldBucketInfo.getStorageType());
- }
-
- //Check Versioning to update
- Boolean versioning = args.getIsVersionEnabled();
- if (versioning != null) {
- bucketInfoBuilder.setIsVersionEnabled(versioning);
- LOG.debug("Updating bucket versioning for bucket: {} in volume: {}",
- bucketName, volumeName);
- } else {
- bucketInfoBuilder
- .setIsVersionEnabled(oldBucketInfo.getIsVersionEnabled());
- }
- bucketInfoBuilder.setCreationTime(oldBucketInfo.getCreationTime());
-
- // Set acls from oldBucketInfo if it has any.
- if (oldBucketInfo.getAcls() != null) {
- bucketInfoBuilder.setAcls(oldBucketInfo.getAcls());
- }
-
- OmBucketInfo omBucketInfo = bucketInfoBuilder.build();
-
-
- commitBucketInfoToDB(omBucketInfo);
- } catch (IOException ex) {
- if (!(ex instanceof OMException)) {
- LOG.error("Setting bucket property failed for bucket:{} in volume:{}",
- bucketName, volumeName, ex);
- }
- throw ex;
- } finally {
- metadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- }
- }
-
- /**
- * Deletes an existing empty bucket from volume.
- *
- * @param volumeName - Name of the volume.
- * @param bucketName - Name of the bucket.
- * @throws IOException - on Failure.
- */
- @Override
- public void deleteBucket(String volumeName, String bucketName)
- throws IOException {
- Preconditions.checkNotNull(volumeName);
- Preconditions.checkNotNull(bucketName);
- metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- //Check if bucket exists
- String bucketKey = metadataManager.getBucketKey(volumeName, bucketName);
- if (metadataManager.getBucketTable().get(bucketKey) == null) {
- LOG.debug("bucket: {} not found ", bucketName);
- throw new OMException("Bucket doesn't exist",
- BUCKET_NOT_FOUND);
- }
- //Check if bucket is empty
- if (!metadataManager.isBucketEmpty(volumeName, bucketName)) {
- LOG.debug("bucket: {} is not empty ", bucketName);
- throw new OMException("Bucket is not empty",
- OMException.ResultCodes.BUCKET_NOT_EMPTY);
- }
- commitDeleteBucketInfoToOMDB(bucketKey);
- } catch (IOException ex) {
- if (!(ex instanceof OMException)) {
- LOG.error("Delete bucket failed for bucket:{} in volume:{}", bucketName,
- volumeName, ex);
- }
- throw ex;
- } finally {
- metadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- }
- }
-
private void commitDeleteBucketInfoToOMDB(String dbBucketKey)
Review comment:
Same here, no longer used.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java
##########
@@ -106,92 +101,6 @@ KeyProviderCryptoExtension getKMSProvider() {
* -> Else update MetadataDB with VolumeInfo.
*/
- /**
- * Creates a bucket.
- *
- * @param bucketInfo - OmBucketInfo.
- */
- @Override
- public void createBucket(OmBucketInfo bucketInfo) throws IOException {
- Preconditions.checkNotNull(bucketInfo);
- String volumeName = bucketInfo.getVolumeName();
- String bucketName = bucketInfo.getBucketName();
- boolean acquiredBucketLock = false;
- metadataManager.getLock().acquireWriteLock(VOLUME_LOCK, volumeName);
- try {
- acquiredBucketLock = metadataManager.getLock().acquireWriteLock(
- BUCKET_LOCK, volumeName, bucketName);
- String volumeKey = metadataManager.getVolumeKey(volumeName);
- String bucketKey = metadataManager.getBucketKey(volumeName, bucketName);
- OmVolumeArgs volumeArgs = metadataManager.getVolumeTable().get(volumeKey);
-
- //Check if the volume exists
- if (volumeArgs == null) {
- LOG.debug("volume: {} not found ", volumeName);
- throw new OMException("Volume doesn't exist",
- VOLUME_NOT_FOUND);
- }
- //Check if bucket already exists
- if (metadataManager.getBucketTable().get(bucketKey) != null) {
- LOG.debug("bucket: {} already exists ", bucketName);
- throw new OMException("Bucket already exist",
- OMException.ResultCodes.BUCKET_ALREADY_EXISTS);
- }
-
- BucketEncryptionKeyInfo bek = bucketInfo.getEncryptionKeyInfo();
-
- boolean hasSourceVolume = bucketInfo.getSourceVolume() != null;
- boolean hasSourceBucket = bucketInfo.getSourceBucket() != null;
-
- if (hasSourceBucket != hasSourceVolume) {
- throw new OMException("Both source volume and source bucket are " +
- "required for bucket links",
- OMException.ResultCodes.INVALID_REQUEST);
- }
-
- if (bek != null && hasSourceBucket) {
- throw new OMException("Encryption cannot be set for bucket links",
- OMException.ResultCodes.INVALID_REQUEST);
- }
-
- BucketEncryptionKeyInfo.Builder bekb =
- createBucketEncryptionKeyInfoBuilder(bek);
-
- OmBucketInfo.Builder omBucketInfoBuilder = bucketInfo.toBuilder()
- .setCreationTime(Time.now());
-
- OzoneAclUtil.inheritDefaultAcls(omBucketInfoBuilder.getAcls(),
- volumeArgs.getDefaultAcls());
-
- if (bekb != null) {
- omBucketInfoBuilder.setBucketEncryptionKey(bekb.build());
- }
-
- OmBucketInfo omBucketInfo = omBucketInfoBuilder.build();
- commitBucketInfoToDB(omBucketInfo);
- if (hasSourceBucket) {
- LOG.debug("created link {}/{} to bucket: {}/{}",
- volumeName, bucketName,
- omBucketInfo.getSourceVolume(), omBucketInfo.getSourceBucket());
- } else {
- LOG.debug("created bucket: {} in volume: {}", bucketName,
- volumeName);
- }
- } catch (IOException ex) {
- if (!(ex instanceof OMException)) {
- LOG.error("Bucket creation failed for bucket:{} in volume:{}",
- bucketName, volumeName, ex);
- }
- throw ex;
- } finally {
- if (acquiredBucketLock) {
- metadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- }
- metadataManager.getLock().releaseWriteLock(VOLUME_LOCK, volumeName);
- }
- }
-
@Nullable
public BucketEncryptionKeyInfo.Builder createBucketEncryptionKeyInfoBuilder(
Review comment:
This method seems to be unused, can we remove it?
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java
##########
@@ -286,118 +195,6 @@ public OmBucketInfo getBucketInfo(String volumeName, String bucketName)
}
}
- /**
- * Sets bucket property from args.
- *
- * @param args - BucketArgs.
- * @throws IOException - On Failure.
- */
- @Override
- public void setBucketProperty(OmBucketArgs args) throws IOException {
- Preconditions.checkNotNull(args);
- String volumeName = args.getVolumeName();
- String bucketName = args.getBucketName();
- metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- try {
- String bucketKey = metadataManager.getBucketKey(volumeName, bucketName);
- OmBucketInfo oldBucketInfo =
- metadataManager.getBucketTable().get(bucketKey);
- //Check if bucket exist
- if (oldBucketInfo == null) {
- LOG.debug("bucket: {} not found ", bucketName);
- throw new OMException("Bucket doesn't exist",
- BUCKET_NOT_FOUND);
- }
- OmBucketInfo.Builder bucketInfoBuilder = OmBucketInfo.newBuilder();
- bucketInfoBuilder.setVolumeName(oldBucketInfo.getVolumeName())
- .setBucketName(oldBucketInfo.getBucketName());
- bucketInfoBuilder.addAllMetadata(args.getMetadata());
-
- //Check StorageType to update
- StorageType storageType = args.getStorageType();
- if (storageType != null) {
- bucketInfoBuilder.setStorageType(storageType);
- LOG.debug("Updating bucket storage type for bucket: {} in volume: {}",
- bucketName, volumeName);
- } else {
- bucketInfoBuilder.setStorageType(oldBucketInfo.getStorageType());
- }
-
- //Check Versioning to update
- Boolean versioning = args.getIsVersionEnabled();
- if (versioning != null) {
- bucketInfoBuilder.setIsVersionEnabled(versioning);
- LOG.debug("Updating bucket versioning for bucket: {} in volume: {}",
- bucketName, volumeName);
- } else {
- bucketInfoBuilder
- .setIsVersionEnabled(oldBucketInfo.getIsVersionEnabled());
- }
- bucketInfoBuilder.setCreationTime(oldBucketInfo.getCreationTime());
-
- // Set acls from oldBucketInfo if it has any.
- if (oldBucketInfo.getAcls() != null) {
- bucketInfoBuilder.setAcls(oldBucketInfo.getAcls());
- }
-
- OmBucketInfo omBucketInfo = bucketInfoBuilder.build();
-
-
- commitBucketInfoToDB(omBucketInfo);
Review comment:
`commitBucketInfoToDB` is also unused.
--
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