Skip to content
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.

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

HDDS-6411. EC: OmMultipartKeyInfo needs to handle ECReplicationConfig #3210

Merged
merged 1 commit into from Mar 22, 2022

Conversation

adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Add ECReplicationConfig in MultipartKeyInfo proto message and its OmMultipartKeyInfo counterpart.

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

How was this patch tested?

Added unit test.

https://github.com/adoroszlai/hadoop-ozone/actions/runs/1997562855

@adoroszlai adoroszlai self-assigned this Mar 17, 2022
Copy link
Contributor

@umamaheswararao umamaheswararao left a comment

Choose a reason for hiding this comment

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

Changes looks good to me.
Since you are at this, I see there is MultipartUploadInfo which has factor too. Do you think we can incorporate here and update JIRA title to reflect. I am ok if you want to file separate JIRA as well.

@adoroszlai
Copy link
Contributor Author

adoroszlai commented Mar 21, 2022

there is MultipartUploadInfo which has factor too. Do you think we can incorporate here

Maybe I'm missing something, but HDDS-6413 already added ECReplicationConfig in MultipartUploadInfo.

message MultipartUploadInfo {
required string volumeName = 1;
required string bucketName = 2;
required string keyName = 3;
required string uploadId = 4;
required uint64 creationTime = 5;
required hadoop.hdds.ReplicationType type = 6;
required hadoop.hdds.ReplicationFactor factor = 7;
optional hadoop.hdds.ECReplicationConfig ecReplicationConfig = 8;
}

if (repType == HddsProtos.ReplicationType.EC) {
bldr.setEcReplicationConfig(
((ECReplicationConfig)upload.getReplicationConfig())
.toProto());
} else {
bldr.setFactor(ReplicationConfig.getLegacyFactor(
upload.getReplicationConfig()));
}

Copy link
Contributor

@umamaheswararao umamaheswararao left a comment

Choose a reason for hiding this comment

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

LGTM

@adoroszlai adoroszlai merged commit 04c62c5 into apache:HDDS-3816-ec Mar 22, 2022
@adoroszlai adoroszlai deleted the HDDS-6411 branch March 22, 2022 18:30
@adoroszlai
Copy link
Contributor Author

Thanks @umamaheswararao for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants