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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]