Skip to content

HDDS-9201. [Ozone-Streaming] Stream copy object support on gateway#5210

Merged
captainzmc merged 6 commits intoapache:masterfrom
guohao-rosicky:guohao-HDDS-9201-dev
Sep 7, 2023
Merged

HDDS-9201. [Ozone-Streaming] Stream copy object support on gateway#5210
captainzmc merged 6 commits intoapache:masterfrom
guohao-rosicky:guohao-HDDS-9201-dev

Conversation

@guohao-rosicky
Copy link
Contributor

@guohao-rosicky guohao-rosicky commented Aug 23, 2023

What changes were proposed in this pull request?

Stream copy object support on gateway.

The current gateway only supports uploading files using craft in the copy object section, this pr uses stream to support the copy object.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing UT

@guohao-rosicky
Copy link
Contributor Author

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks @guohao-rosicky for working on this.

  1. PR/Jira description is missing the motivation behind the change. It would be great if you add more details there.
  2. I doubt if existing tests are sufficient. We might not be reaching here: https://github.com/apache/ozone/pull/5210/files#diff-bd6abd262ed8e24eaeea5cb0e7c2a1a128411b284f9f27dae1ad47be6717a940R1011 based on current tests. Please correct me if we already have test for it.

volume.getName(), destBucket, destKey, srcKeyLen,
replication, metadata)) {
copyLength = IOUtils.copyLarge(src, dest);
if (datastreamEnabled && !((replication != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (datastreamEnabled && !((replication != null &&
if (datastreamEnabled && !(replication != null &&
replication.getReplicationType() == EC) &&
srcKeyLen > datastreamMinLength) {

Please remove redundant parentheses.

@guohao-rosicky
Copy link
Contributor Author

guohao-rosicky commented Aug 30, 2023

Thanks @hemantk-12 for the review.
Added new ut & all ci is passed.
please see: https://github.com/guohao-rosicky/ozone/actions/runs/6019656855

@guohao-rosicky
Copy link
Contributor Author

guohao-rosicky commented Aug 30, 2023

  1. PR/Jira description is missing the motivation behind the change. It would be great if you add more details there.

hi, @hemantk-12, The description has been updated

Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@captainzmc captainzmc left a comment

Choose a reason for hiding this comment

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

The change looks good. Wait for CI execution.

@guohao-rosicky
Copy link
Contributor Author

guohao-rosicky commented Sep 6, 2023

Thanks @captainzmc for the review, all ci passed.

@captainzmc captainzmc merged commit eb0836e into apache:master Sep 7, 2023
@captainzmc
Copy link
Member

Thanks @guohao-rosicky for the patch. And thanks @hemantk-12 for the review. Let's merge this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments