Skip to content

HDDS-10630. Add missing parent directories deleted between initiate and complete MPU - #6496

Merged
adoroszlai merged 7 commits into
apache:masterfrom
SaketaChalamchala:HDDS-10630
Apr 12, 2024
Merged

HDDS-10630. Add missing parent directories deleted between initiate and complete MPU#6496
adoroszlai merged 7 commits into
apache:masterfrom
SaketaChalamchala:HDDS-10630

Conversation

@SaketaChalamchala

@SaketaChalamchalaSaketaChalamchala commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

S3a provides multiple mapreduce committers. When using the directory staging committer fs.s3a.committer.name=directory with replace conflict mode fs.s3a.committer.staging.conflict-mode=replace and writing to FSO buckets, the job fails with the following errors.

This is because the logic to add back missing parent directories and missing output prefix directories similar to Initiate MPU request (S3InitiateMultipartUploadRequestWithFSO.java, S3InitiateMultipartUploadResponseWithFSO.java) is missing in the Complete MPU request.

The proposed solution adds the missing parent and output prefix directories back to DB before completing the MPU.

Errors:

##When creating getDBOzoneKey
StateMachine ApplyTransaction Thread - 0]-org.apache.hadoop.ozone.om.request.s3.multipart.S3MultipartUploadCompleteRequest: MultipartUpload Complete request failed for Key: st-data-con-jqmmwt/qetest/terasort/output-1710494153/part-r-00000 in Volume/Bucket s3v/qe-dataconn-bucket
DIRECTORY_NOT_FOUND org.apache.hadoop.ozone.om.exceptions.OMException: Failed to find parent directory of st-data-con-jqmmwt/qetest/terasort/output-1710494153/part-r-00000
at org.apache.hadoop.ozone.om.request.file.OMFileRequest.getParentID(OMFileRequest.java:1008)
at org.apache.hadoop.ozone.om.request.file.OMFileRequest.getParentID(OMFileRequest.java:958)
at org.apache.hadoop.ozone.om.request.file.OMFileRequest.getParentId(OMFileRequest.java:1038)
at org.apache.hadoop.ozone.om.request.s3.multipart.S3MultipartUploadCompleteRequestWithFSO.getDBOzoneKey(S3MultipartUploadCompleteRequestWithFSO.java:114)
at org.apache.hadoop.ozone.om.request.s3.multipart.S3MultipartUploadCompleteRequest.validateAndUpdateCache(S3MultipartUploadCompleteRequest.java:157)
at org.apache.hadoop.ozone.protocolPB.OzoneManagerRequestHandler.handleWriteRequest(OzoneManagerRequestHandler.java:378)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.runCommand(OzoneManagerStateMachine.java:568)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.lambda$1(OzoneManagerStateMachine.java:363)
at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1700)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
## When getting omKeyInfo from DB
StateMachine ApplyTransaction Thread - 0]-org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine: Terminating with exit status 1: Request cmdType: CompleteMultiPartUpload
traceID: ""
clientId: "client-5168AA460706"
userInfo {
userName: "xxx"
remoteAddress: "..."
hostName: "..."
}
version: 3
completeMultiPartUploadRequest {
keyArgs {
volumeName: "s3v"
bucketName: "qe-dataconn-bucket"
keyName: "st-data-con-jqmmwt/qetest/terasort/output-1710494153/part-r-00000"
multipartUploadID: "e1c08d2c-5798-4c81-ab2c-7a0bd0fea4c9-112101950169613319"
acls {
type: USER
name: "..."
rights: "\200"
aclScope: ACCESS
}
acls {
type: GROUP
name: "hrt_qa"
rights: "\000\001"
aclScope: ACCESS
}
acls {
type: GROUP
name: "users"
rights: "\000\001"
aclScope: ACCESS
}
acls {
type: GROUP
name: "hivetest"
rights: "\000\001"
aclScope: ACCESS
}
modificationTime: 1710540186541
}
partsList {
partNumber: 1
partName: "/s3v/qe-dataconn-bucket/st-data-con-jqmmwt/qetest/terasort/output-1710494153/part-r-00000-e1c08d2c-5798-4c81-ab2c-7a0bd0fea4c9-112101950169613319-1"
}
}
s3Authentication {
stringToSign: "..."
signature: "..."
accessId: "..."
}
failed with exception
java.lang.NullPointerException
at org.apache.hadoop.ozone.om.request.s3.multipart.S3MultipartUploadCompleteRequest.getOmKeyInfo(S3MultipartUploadCompleteRequest.java:378)
at org.apache.hadoop.ozone.om.request.s3.multipart.S3MultipartUploadCompleteRequest.validateAndUpdateCache(S3MultipartUploadCompleteRequest.java:202)
at org.apache.hadoop.ozone.protocolPB.OzoneManagerRequestHandler.handleWriteRequest(OzoneManagerRequestHandler.java:378)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.runCommand(OzoneManagerStateMachine.java:568)
at org.apache.hadoop.ozone.om.ratis.OzoneManagerStateMachine.lambda$1(OzoneManagerStateMachine.java:363)
at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1700)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834) 

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10630

How was this patch tested?

Ran the s3a hadoop contract test ITestS3ACommitterMRJob to verify. There is another PR out to add s3a contract tests to acceptance testing.

 ## Startup unsecure ozone cluster using docker-compose and create an FSO bucket
cd hadoop-ozone/dist/target/ozone-*-SNAPSHOT/compose/ozone
docker-compose up -d --scale datanode=3
ozone sh bucket create /s3v/fso-bucket -l FILE_SYSTEM_OPTIMIZED
## Download the hadoop-aws source
curl -LSs -o "hadoop-src.tar.gz" https://archive.apache.org/dist/hadoop/common/hadoop-3.3.6/hadoop-3.3.6-src.tar.gz
tar -x -z -C "hadoop-src" --strip-components=3 -f "hadoop-src.tar.gz" 'hadoop-*-src/hadoop-tools/hadoop-aws'
## Create auth-keys.xml
vi hadoop-src/src/test/resources/auth-keys.xml <configuration>
<property>
<name>fs.s3a.endpoint</name>
<value>http://localhost:9878</value>
</property>
<property>
<name>fs.s3a.access.key</name>
<value>s3a-contract</value>
</property>
<property>
<name>fs.s3a.secret.key</name>
<value>unsecure</value>
</property>
<property>
<name>fs.s3a.committer.staging.conflict-mode</name>
<value>replace</value>
</property>
<property>
<name>fs.contract.test.fs.s3a</name>
<value>s3a://fso-bucket/</value>
</property>
<property>
<name>test.fs.s3a.name</name>
<value>s3a://fso-bucket/</value>
</property>
<property>
<name>test.fs.s3a.sts.enabled</name>
<value>false</value>
</property>
<property>
<name>fs.s3a.path.style.access</name>
<value>true</value>
</property>
<property>
<name>fs.s3a.directory.marker.retention</name>
<value>keep</value>
</property>
</configuration>
## Run the ITestS3ACommitterMRJob contract test
mvn clean test -B -V --no-transfer-progress -Dtest='ITestS3ACommitterMRJob' 

@adoroszlaiadoroszlai added the s3 S3 Gateway label Apr 8, 2024

@adoroszlaiadoroszlai left a comment

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.

Thanks @SaketaChalamchala for the patch. Functionality looks OK to me. Please take a look at failing unit test. Would be nice to reuse some code from S3InitiateMultipartUploadRequestWithFSO.

@SaketaChalamchala

Copy link
Copy Markdown
ContributorAuthor

Thanks for the review @adoroszlai. The unit tests have been fixed.
The CI is running on my fork: https://github.com/SaketaChalamchala/ozone/actions/runs/8609322147. There's just a recon test that's timing out. Retrying it.

@adoroszlai
adoroszlai requested review from ashishkumar50 and removed request for ashishkumar50April 9, 2024 08:42
Comment on lines +95 to +114
// Create missing parent directory entries.
try (BatchOperation batchOperation = omMetadataManager.getStore()
.initBatchOperation()) {
for (OmDirectoryInfo parentDirInfo : missingParentInfos) {
final String parentKey = omMetadataManager.getOzonePathKey(
volumeId, bucketId, parentDirInfo.getParentObjectID(),
parentDirInfo.getName());
omMetadataManager.getDirectoryTable().putWithBatch(batchOperation,
parentKey, parentDirInfo);
}

// namespace quota changes for parent directory
String bucketKey = omMetadataManager.getBucketKey(
omBucketInfo.getVolumeName(),
omBucketInfo.getBucketName());
omMetadataManager.getBucketTable().putWithBatch(batchOperation,
bucketKey, omBucketInfo);

omMetadataManager.getStore().commitBatchOperation(batchOperation);
}

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.

I think adding to DB batch should be handled in the response object by design. It uses an existing batch and commits only if the response is OK.

@Override
publicvoidaddToDBBatch(OMMetadataManageromMetadataManager,
BatchOperationbatchOperation) throwsIOException {
/**
* Create parent directory entries during MultiPartFileKey Create - do not
* wait for File Commit request.
*/
if (parentDirInfos != null) {
for (OmDirectoryInfoparentDirInfo : parentDirInfos) {
finalStringparentKey = omMetadataManager.getOzonePathKey(
volumeId, bucketId, parentDirInfo.getParentObjectID(),
parentDirInfo.getName());
omMetadataManager.getDirectoryTable().putWithBatch(batchOperation,
parentKey, parentDirInfo);
}
// namespace quota changes for parent directory
StringbucketKey = omMetadataManager.getBucketKey(
omBucketInfo.getVolumeName(),
omBucketInfo.getBucketName());
omMetadataManager.getBucketTable().putWithBatch(batchOperation,
bucketKey, omBucketInfo);
}

@ashishkumar50ashishkumar50 left a comment

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.

@SaketaChalamchala Thanks for the patch, Please find the comment below.
Only cache entry should be added in request and db insert should be done in response.

final String parentKey = omMetadataManager.getOzonePathKey(
volumeId, bucketId, parentDirInfo.getParentObjectID(),
parentDirInfo.getName());
omMetadataManager.getDirectoryTable().putWithBatch(batchOperation,

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.

I think this should be done in S3MultipartUploadCompleteResponseWithFSO instead here or else db entry will be added only on the leader OM.

try (BatchOperation batchOperation = omMetadataManager.getStore()
.initBatchOperation()) {

OMFileRequest.addToOpenFileTableForMultipart(omMetadataManager,

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.

Even this should be done in S3MultipartUploadCompleteResponseWithFSO instead here.

@SaketaChalamchala

Copy link
Copy Markdown
ContributorAuthor

@adoroszlai and @ashishkumar50 updated the patch based on your comments. Please let me know if you have any more comments.

@adoroszlaiadoroszlai left a comment

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.

Thanks @SaketaChalamchala for updating the patch, LGTM. Also tested with ITestS3ACommitterMRJob.

@adoroszlaiadoroszlai changed the title HDDS-10630. Add missing parent and prefix directories that were deleted between commit and complete mpu uploads.HDDS-10630. Add missing parent directories deleted between initiate and complete MPUApr 11, 2024

@ashishkumar50ashishkumar50 left a comment

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.

@SaketaChalamchala Thanks for updating patch, Change LGTM.

@adoroszlai
adoroszlai merged commit c1b27a8 into apache:masterApr 12, 2024
@adoroszlai

Copy link
Copy Markdown
Contributor

Thanks @SaketaChalamchala for the patch, @ashishkumar50 for the review.

Tejaskriya pushed a commit to Tejaskriya/ozone that referenced this pull request Apr 17, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Apr 17, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Apr 18, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Apr 18, 2024
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Apr 23, 2024
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Jun 10, 2024
…s deleted between initiate and complete MPU (apache#6496)
(cherry picked from commit c1b27a8)
Change-Id: I365ea43497bf47d5c00570c665f2c293e73f4c09
Comment on lines +97 to +101
final String parentKey = omMetadataManager.getOzonePathKey(
volumeId, bucketId, parentDirInfo.getParentObjectID(),
parentDirInfo.getName());
omMetadataManager.getDirectoryTable().putWithBatch(batchOperation,
parentKey, parentDirInfo);

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.

Correct me if I'm wrong, but since we are adding to the directoryTable, it needs to be added in the @CleanupTableInfo.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

s3S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@SaketaChalamchala@adoroszlai@ivandika3@ashishkumar50