This is an automated email from the ASF dual-hosted git repository.
pifta 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 51a5fb9422 Revert "HDDS-11235. Spare InfoBucket RPC call in
FileSystem#mkdir() call. (#6990)" (#7122)
51a5fb9422 is described below
commit 51a5fb9422108e47ea10d7118f1ee9baec4a7e3b
Author: tanvipenumudy <[email protected]>
AuthorDate: Wed Aug 28 03:34:08 2024 +0530
Revert "HDDS-11235. Spare InfoBucket RPC call in FileSystem#mkdir() call.
(#6990)" (#7122)
---
.../apache/hadoop/ozone/client/rpc/RpcClient.java | 2 -
.../ozone/BasicRootedOzoneClientAdapterImpl.java | 121 ++++++++++-----------
2 files changed, 58 insertions(+), 65 deletions(-)
diff --git
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
index f01fddf40f..35db51b3e4 100644
---
a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
+++
b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -2164,8 +2164,6 @@ public class RpcClient implements ClientProtocol {
@Override
public void createDirectory(String volumeName, String bucketName,
String keyName) throws IOException {
- verifyVolumeName(volumeName);
- verifyBucketName(bucketName);
String ownerName = getRealUserInfo().getShortUserName();
OmKeyArgs keyArgs = new OmKeyArgs.Builder().setVolumeName(volumeName)
.setBucketName(bucketName)
diff --git
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
index 14c297d9f4..da278f17fb 100644
---
a/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
+++
b/hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
@@ -260,8 +260,11 @@ public class BasicRootedOzoneClientAdapterImpl
}
}
- OzoneBucket getBucket(OFSPath ofsPath, boolean createIfNotExist)throws
IOException {
- return getBucket(ofsPath.getVolumeName(), ofsPath.getBucketName(),
createIfNotExist);
+ OzoneBucket getBucket(OFSPath ofsPath, boolean createIfNotExist)
+ throws IOException {
+
+ return getBucket(ofsPath.getVolumeName(), ofsPath.getBucketName(),
+ createIfNotExist);
}
/**
@@ -273,7 +276,8 @@ public class BasicRootedOzoneClientAdapterImpl
* @throws IOException Exceptions other than OMException with result code
* VOLUME_NOT_FOUND or BUCKET_NOT_FOUND.
*/
- private OzoneBucket getBucket(String volumeStr, String bucketStr, boolean
createIfNotExist) throws IOException {
+ private OzoneBucket getBucket(String volumeStr, String bucketStr,
+ boolean createIfNotExist) throws IOException {
Preconditions.checkNotNull(volumeStr);
Preconditions.checkNotNull(bucketStr);
@@ -283,7 +287,7 @@ public class BasicRootedOzoneClientAdapterImpl
"getBucket: Invalid argument: given bucket string is empty.");
}
- OzoneBucket bucket = null;
+ OzoneBucket bucket;
try {
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
@@ -295,8 +299,44 @@ public class BasicRootedOzoneClientAdapterImpl
OzoneFSUtils.validateBucketLayout(bucket.getName(),
resolvedBucketLayout);
} catch (OMException ex) {
if (createIfNotExist) {
- handleVolumeOrBucketCreationOnException(volumeStr, bucketStr, ex);
- // Try to get the bucket again
+ // getBucketDetails can throw VOLUME_NOT_FOUND when the parent volume
+ // doesn't exist and ACL is enabled; it can only throw BUCKET_NOT_FOUND
+ // when ACL is disabled. Both exceptions need to be handled.
+ switch (ex.getResult()) {
+ case VOLUME_NOT_FOUND:
+ // Create the volume first when the volume doesn't exist
+ try {
+ objectStore.createVolume(volumeStr);
+ } catch (OMException newVolEx) {
+ // Ignore the case where another client created the volume
+ if (!newVolEx.getResult().equals(VOLUME_ALREADY_EXISTS)) {
+ throw newVolEx;
+ }
+ }
+ // No break here. Proceed to create the bucket
+ case BUCKET_NOT_FOUND:
+ // When BUCKET_NOT_FOUND is thrown, we expect the parent volume
+ // exists, so that we don't call create volume and incur
+ // unnecessary ACL checks which could lead to unwanted behavior.
+ OzoneVolume volume = proxy.getVolumeDetails(volumeStr);
+ // Create the bucket
+ try {
+ // Buckets created by OFS should be in FSO layout
+ volume.createBucket(bucketStr,
+ BucketArgs.newBuilder().setBucketLayout(
+ this.defaultOFSBucketLayout).build());
+ } catch (OMException newBucEx) {
+ // Ignore the case where another client created the bucket
+ if (!newBucEx.getResult().equals(BUCKET_ALREADY_EXISTS)) {
+ throw newBucEx;
+ }
+ }
+ break;
+ default:
+ // Throw unhandled exception
+ throw ex;
+ }
+ // Try get bucket again
bucket = proxy.getBucketDetails(volumeStr, bucketStr);
} else {
throw ex;
@@ -306,41 +346,6 @@ public class BasicRootedOzoneClientAdapterImpl
return bucket;
}
- private void handleVolumeOrBucketCreationOnException(String volumeStr,
String bucketStr, OMException ex)
- throws IOException {
- // OM can throw VOLUME_NOT_FOUND when the parent volume does not exist,
and in this case we may create the volume,
- // OM can also throw BUCKET_NOT_FOUND when the parent bucket does not
exist, and so we also may create the bucket.
- // This method creates the volume and the bucket when an exception marks
that they don't exist.
- switch (ex.getResult()) {
- case VOLUME_NOT_FOUND:
- // Create the volume first when the volume doesn't exist
- try {
- objectStore.createVolume(volumeStr);
- } catch (OMException newVolEx) {
- // Ignore the case where another client created the volume
- if (!newVolEx.getResult().equals(VOLUME_ALREADY_EXISTS)) {
- throw newVolEx;
- }
- }
- // No break here. Proceed to create the bucket
- case BUCKET_NOT_FOUND:
- // Create the bucket
- try {
- // Buckets created by OFS should be in FSO layout
- BucketArgs defaultBucketArgs =
BucketArgs.newBuilder().setBucketLayout(this.defaultOFSBucketLayout).build();
- proxy.createBucket(volumeStr, bucketStr, defaultBucketArgs);
- } catch (OMException newBucEx) {
- // Ignore the case where another client created the bucket
- if (!newBucEx.getResult().equals(BUCKET_ALREADY_EXISTS)) {
- throw newBucEx;
- }
- }
- break;
- default:
- throw ex;
- }
- }
-
/**
* This API returns the value what is configured at client side only. It
could
* differ from the server side default values. If no replication config
@@ -510,40 +515,30 @@ public class BasicRootedOzoneClientAdapterImpl
LOG.trace("creating dir for path: {}", pathStr);
incrementCounter(Statistic.OBJECTS_CREATED, 1);
OFSPath ofsPath = new OFSPath(pathStr, config);
-
- String volumeName = ofsPath.getVolumeName();
- if (volumeName.isEmpty()) {
+ if (ofsPath.getVolumeName().isEmpty()) {
// Volume name unspecified, invalid param, return failure
return false;
}
-
- String bucketName = ofsPath.getBucketName();
- if (bucketName.isEmpty()) {
- // Create volume only as path only contains one element the volume.
- objectStore.createVolume(volumeName);
+ if (ofsPath.getBucketName().isEmpty()) {
+ // Create volume only
+ objectStore.createVolume(ofsPath.getVolumeName());
return true;
}
-
String keyStr = ofsPath.getKeyName();
try {
- if (keyStr == null || keyStr.isEmpty()) {
- // This is the case when the given path only contains volume and
bucket.
- // If the bucket does not exist, then this will throw and bucket will
be created
- // in handleVolumeOrBucketCreationOnException later.
- proxy.getBucketDetails(volumeName, bucketName);
- } else {
- proxy.createDirectory(volumeName, bucketName, keyStr);
+ OzoneBucket bucket = getBucket(ofsPath, true);
+ // Empty keyStr here indicates only volume and bucket is
+ // given in pathStr, so getBucket above should handle the creation
+ // of volume and bucket. We won't feed empty keyStr to
+ // bucket.createDirectory as that would be a NPE.
+ if (keyStr != null && keyStr.length() > 0) {
+ bucket.createDirectory(keyStr);
}
} catch (OMException e) {
if (e.getResult() == OMException.ResultCodes.FILE_ALREADY_EXISTS) {
throw new FileAlreadyExistsException(e.getMessage());
}
- // Create volume and bucket if they do not exist, and retry key creation.
- // This call will throw an exception if it fails, or the exception is
different than it handles.
- handleVolumeOrBucketCreationOnException(volumeName, bucketName, e);
- if (keyStr != null && !keyStr.isEmpty()) {
- proxy.createDirectory(volumeName, bucketName, keyStr);
- }
+ throw e;
}
return true;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
