HDDS-15182. [STS] Remove latent extra read on CopyObject api - make CopyPart work similarly by fmorg-git · Pull Request #10203 · apache/ozone

fmorg-git · 2026-05-06T19:07:29Z

Please describe your PR in detail:

In STS, CopyObject requires s3:GetObject action on source and s3:PutObject action on destination in order to successfully invoke the api. In the latent S3 implementation, the code was doing an additional read on the destination object to get the eTag and modificationTime when the file was created. This is problematic, because AWS doesn't require s3:GetObject action on the destination file, so this extra read was causing STS smoke tests to fail.

In order to address, don't do the extra read (which will help performance as well) and instead make the commit key call return the modification time in the response and use that. Furthermore, the code already knows what the md5Hash/eTag is, so there is no need to read the destination object.

For consistency, make uploadPartCopy similarly return the modification time as well.

What is the link to the Apache JIRA

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

How was this patch tested?

unit tests, smoke tests

github-actions · 2026-06-07T00:26:46Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

fmorg-git · 2026-06-07T22:44:13Z

commenting to remove stale label

…arly Conflicts: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

fmorg-git · 2026-06-12T10:18:32Z

hi @ChenSammi - this PR has been rebased and ready to review once the workflow checks pass. Thanks!

ChenSammi · 2026-06-16T03:48:28Z

In the latent S3 implementation, the code was doing an additional read on the destination object to get the eTag and modificationTime when the file was created.

Fabian, Can you point to the code which is doing this?

This is problematic, because AWS doesn't require s3:GetObject action on the destination file, so this extra read was causing STS smoke tests to fail

Can you point out failed smoke test?

ChenSammi · 2026-06-16T04:26:09Z

+   * This is backward compatible with older OM versions which do not return
+   * the value in {@code CommitKeyResponse}.
+   */
+  default OptionalLong commitKeyWithModificationTime(OmKeyArgs args, long clientID) throws IOException {


It looks like that LastModified element is a mandatory part of CopyObject and UploadPartCopy.

https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html
https://docs.aws.amazon.com/AmazonS3/latest/API/API_UploadPartCopy.html

So without this patch, Ozone now returns incorrect LastModified? Since there is already lastModified field in CopyObjectResponse and CopyPartResult.

Since it's mandatory for S3, we don't need to maintain both commitKeyWithModificationTime and commitKey, update commitKey to return long, instead of OptionalLong, making sure lastModified is returned after a successful commit, so we can get a clean logic and code.

So without this patch, Ozone now returns incorrect LastModified?

There were two cases previously per my understanding:

CopyObject: the current time was set when the key is committed. Then an extra lookup is performed in getClientProtocol().getKeyDetails() to find what that modification time is (so it was correct). This patch changes to return the modification time in the CommitKeyResponse itself so the extra lookup isn't needed (and this extra lookup was breaking STS).

UploadPartCopy: it was just doing an Instant.now() in the CopyPartResult constructor, so the returned LastModified was the gateway's wall‑clock at response time, not the OM‑side commit time of the part key, so it was not correct but was approximate

For both these cases, the patch now returns the actual lastModified that was set at the time the key was committed in the response itself so that a) no extra fetch is needed and b) it is more accurate

Since it's mandatory for S3, we don't need to maintain both commitKeyWithModificationTime and commitKey, update commitKey to return long,

@ChenSammi - would this be a breaking api change to commitKey for older clients or mixed client versions?

ChenSammi · 2026-06-16T04:31:15Z

  EchoRPCResponse echoRPCReq(byte[] payloadReq, int payloadSizeResp,
                             boolean writeToRatis) throws IOException;

-


This can be reverted.

ChenSammi · 2026-06-16T04:32:19Z

          {
-            "name": "CommitKeyResponse"
+            "name": "CommitKeyResponse",
+            "fields": [


proto.lock change doesn't need be included in patch. It will be updated as a whole during Ozone release.

proto.lock change doesn't need be included in patch. It will be updated as a whole during Ozone release.

Small, but important correction: proto.lock shouldn't be changed (in regular PRs).

updated - cd3d5b2

ChenSammi · 2026-06-16T04:35:26Z

@@ -46,8 +46,12 @@ public CopyPartResult() {
  }

  public CopyPartResult(String eTag) {


Remove this constructor if it's not used anymore.

ChenSammi · 2026-06-16T04:36:23Z

+  private final Long modificationTime;
+
  public OmMultipartCommitUploadPartInfo(String partName, String eTag) {
+    this(partName, eTag, null);


Remove this constructor if it's not used anymore.

ChenSammi · 2026-06-16T04:39:30Z

-        omClient.commitKey(buildKeyArgs(), openID);
+        final OptionalLong modificationTime = omClient.commitKeyWithModificationTime(buildKeyArgs(), openID);
+        if (modificationTime.isPresent()) {
+          metadata.put(OzoneConsts.MODIFICATION_TIME, String.valueOf(modificationTime.getAsLong()));


metadata is to persist in OM DB, a different goal. Please add a new field for LastModified value.

ChenSammi · 2026-06-16T04:41:58Z

@fabian, could you also add some tests for the new changes?

adoroszlai · 2026-06-16T04:55:54Z

          {
-            "name": "CommitKeyResponse"
+            "name": "CommitKeyResponse",
+            "fields": [


proto.lock change doesn't need be included in patch. It will be updated as a whole during Ozone release.

Small, but important correction: proto.lock shouldn't be changed (in regular PRs).

fmorg-git · 2026-06-17T00:37:43Z

In the latent S3 implementation, the code was doing an additional read on the destination object to get the eTag and modificationTime when the file was created.

Fabian, Can you point to the code which is doing this?

This is problematic, because AWS doesn't require s3:GetObject action on the destination file, so this extra read was causing STS smoke tests to fail

Can you point out failed smoke test?

I removed the code that was doing the additional read on the destination object. Copilot flagged it as an issue in the previous PR as well but had an incorrect suggested fix: #10197 (comment) (open this link in a new tab for it to go to the correct comment)

This series of smoke tests will fail without this fix: https://github.com/fmorg-git/ozone/blob/0ece11555841b68822e0f5a87c77fc4830ec4fda/hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-sts.robot#L927-L984

fmorg-git changed the base branch from master to HDDS-13323-sts May 6, 2026 19:07

fmorg-git force-pushed the HDDS-15182 branch from 4fe6c22 to 7705dd4 Compare May 6, 2026 19:13

fmorg-git marked this pull request as draft May 6, 2026 19:36

fmorg-git force-pushed the HDDS-15182 branch from 7705dd4 to 9e8b9bd Compare May 6, 2026 21:42

fmorg-git mentioned this pull request May 14, 2026

HDDS-14810. [STS] Part 2 - STS Ranger Smoke Tests #9902

Draft

fmorg-git force-pushed the HDDS-15182 branch 3 times, most recently from fdbfdd7 to a6364b3 Compare May 16, 2026 02:21

github-actions Bot added the stale label Jun 7, 2026

github-actions Bot removed the stale label Jun 8, 2026

remove latent extra read on CopyObject api - make CopyPart work simil…

c6b10fb

…arly Conflicts: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

fmorg-git force-pushed the HDDS-15182 branch from a6364b3 to c6b10fb Compare June 12, 2026 10:17

fmorg-git marked this pull request as ready for review June 12, 2026 10:17

ChenSammi reviewed Jun 16, 2026

View reviewed changes

adoroszlai requested changes Jun 16, 2026

View reviewed changes

pr review comment from adoroszlai - revert proto.lock change

cd3d5b2

		EchoRPCResponse echoRPCReq(byte[] payloadReq, int payloadSizeResp,
		boolean writeToRatis) throws IOException;

		@@ -46,8 +46,12 @@ public CopyPartResult() {
		}

		public CopyPartResult(String eTag) {

Conversation

fmorg-git commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

github-actions Bot commented Jun 7, 2026

Uh oh!

fmorg-git commented Jun 7, 2026

Uh oh!

fmorg-git commented Jun 12, 2026

Uh oh!

ChenSammi commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChenSammi Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmorg-git Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChenSammi commented Jun 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmorg-git commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fmorg-git commented May 6, 2026 •

edited

Loading

ChenSammi commented Jun 16, 2026 •

edited

Loading

ChenSammi Jun 16, 2026 •

edited

Loading

fmorg-git Jun 17, 2026 •

edited

Loading

fmorg-git commented Jun 17, 2026 •

edited

Loading