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 97a7ac4c629f..ab7a110723d4 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 @@ -2169,6 +2169,8 @@ public OzoneFileStatus getOzoneFileStatus(String volumeName, @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 109e19d13c9f..59c7291c49ab 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 @@ -241,11 +241,8 @@ private void initDefaultFsBucketLayout(OzoneConfiguration conf) } } - 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); } /** @@ -257,8 +254,7 @@ OzoneBucket getBucket(OFSPath ofsPath, boolean createIfNotExist) * @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); @@ -268,7 +264,7 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, "getBucket: Invalid argument: given bucket string is empty."); } - OzoneBucket bucket; + OzoneBucket bucket = null; try { bucket = proxy.getBucketDetails(volumeStr, bucketStr); @@ -280,44 +276,8 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, OzoneFSUtils.validateBucketLayout(bucket.getName(), resolvedBucketLayout); } catch (OMException ex) { if (createIfNotExist) { - // 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 + handleVolumeOrBucketCreationOnException(volumeStr, bucketStr, ex); + // Try to get the bucket again bucket = proxy.getBucketDetails(volumeStr, bucketStr); } else { throw ex; @@ -327,6 +287,41 @@ private OzoneBucket getBucket(String volumeStr, String bucketStr, 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 @@ -496,30 +491,40 @@ public boolean createDirectory(String pathStr) throws IOException { LOG.trace("creating dir for path: {}", pathStr); incrementCounter(Statistic.OBJECTS_CREATED, 1); OFSPath ofsPath = new OFSPath(pathStr, config); - if (ofsPath.getVolumeName().isEmpty()) { + + String volumeName = ofsPath.getVolumeName(); + if (volumeName.isEmpty()) { // Volume name unspecified, invalid param, return failure return false; } - if (ofsPath.getBucketName().isEmpty()) { - // Create volume only - objectStore.createVolume(ofsPath.getVolumeName()); + + String bucketName = ofsPath.getBucketName(); + if (bucketName.isEmpty()) { + // Create volume only as path only contains one element the volume. + objectStore.createVolume(volumeName); return true; } + String keyStr = ofsPath.getKeyName(); try { - 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); + 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); } } catch (OMException e) { if (e.getResult() == OMException.ResultCodes.FILE_ALREADY_EXISTS) { throw new FileAlreadyExistsException(e.getMessage()); } - throw e; + // 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); + } } return true; }