Skip to content

HDDS-11235. Spare InfoBucket RPC call in FileSystem#mkdir() call. #6990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fapifta merged 4 commits into from
Aug 5, 2024
Merged

HDDS-11235. Spare InfoBucket RPC call in FileSystem#mkdir() call. #6990

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04c09a9
HDDS-11235. Spare InfoBucket RPC call in FileSystem#mkdir() call.
fapifta Jul 26, 2024
3290394
Add logic to create non-existent volumes/buckets to directory creation.
fapifta Jul 26, 2024
851d3cb
Provide a better approach to completely circumvent getBucket call, th…
fapifta Jul 26, 2024
630ff7a
One more additional fix, to do not create directory if the key str is…
fapifta Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 16 additions & 10 deletions ...fs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,24 +496,30 @@ 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()) {

String bucketName = ofsPath.getBucketName();
if (bucketName.isEmpty()) {
// Create volume only
objectStore.createVolume(ofsPath.getVolumeName());
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()) {
// 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.
getBucket(volumeName, bucketName, true);
} else {
proxy.createDirectory(volumeName, bucketName, keyStr);
Comment on lines +515 to +516

@kerneltime kerneltime Jul 26, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this code expected to create both the bucket and the path? The code before would issue a redundant get bucket info if the bucket was already there but the new code will fail if the bucket is not present?

@fapifta fapifta Jul 26, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right, I missed that property of the getBucket call, I have changed the patch, to see if this was the only cause for the test failures. I will update the PR description once things settle about the new changes.

}
} catch (OMException e) {
if (e.getResult() == OMException.ResultCodes.FILE_ALREADY_EXISTS) {
Expand Down