Skip to content

HDDS-9136. Throw exception when rename fails during moveToTrash.#5253

Merged
ChenSammi merged 6 commits intoapache:masterfrom
ashishkumar50:HDDS-9136
Sep 13, 2023
Merged

HDDS-9136. Throw exception when rename fails during moveToTrash.#5253
ChenSammi merged 6 commits intoapache:masterfrom
ashishkumar50:HDDS-9136

Conversation

@ashishkumar50
Copy link
Contributor

What changes were proposed in this pull request?

moveToTrash is not checking for return value of rename API. When rename fails due to various reasons, still moveToTrash assume it as success. This leads to inconsistent behaviour, one which is mentioned in Jira description(HDDS-9136).
In this PR, If there is failure in rename operation, we are throwing exception to know moveToTrash has failed.

What is the link to the Apache JIRA

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

How was this patch tested?

Manually tested.

@ashishkumar50
Copy link
Contributor Author

@sadanand48 @jojochuang Please help to review, thanks.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

wait for the precommit test. I suspect this change'll break the contract tests.

Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

the file systems has the same handling for FSO buckets already. This PR proposes the same change for OBS/Legacy buckets

@sadanand48 sadanand48 changed the title HDDS 9136. Throw exception when rename fails during moveToTrash HDDS-9136. Throw exception when rename fails during moveToTrash. Sep 7, 2023
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 for the patch @ashishkumar50

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.

Overall changes look good to me.

Is it possible to have some kind of unit or integration test for it? If yes, please add them.

@ashishkumar50
Copy link
Contributor Author

Overall changes look good to me.

Is it possible to have some kind of unit or integration test for it? If yes, please add them.

I tried but it's difficult to have a test for this.

@ChenSammi ChenSammi merged commit 11c46c6 into apache:master Sep 13, 2023
errose28 added a commit to errose28/ozone that referenced this pull request Sep 14, 2023
* master: (55 commits)
  HDDS-9236. Fix snapdiff output for key modification (apache#5258)
  HDDS-8013. Freon S3 bucket creation test should use unique prefix (apache#5282)
  HDDS-9228. Poor S3G read performance (apache#5274)
  HDDS-8941. Disable flaky TestContainerBalancerTask#testDelayedStart
  HDDS-1159. Remove flaky tag from TestContainerStateManagerIntegration (apache#5291)
  HDDS-6077. Remove flaky tag from TestAddRemoveOzoneManager (apache#5290)
  HDDS-6610. Remove support for recursive volume list/delete using ozone fs command (apache#5264)
  HDDS-7752. GetS3SecretRequest API should not return secret if secret of user already exists (apache#4538)
  HDDS-9173. Invalidate snapshot cache once snapshot gets purged (apache#5248)
  HDDS-8920. Ozone is supporting unicode volume and bucket names, unintentionally (apache#5276)
  HDDS-9275. LegacyReplicationManager: Delete excess unhealthy with force=true (apache#5286)
  HDDS-9264. Execute EC acceptance test in secure environment (apache#5279)
  HDDS-9161. Recon Pipelines datanode columns search does not work (apache#5213)
  HDDS-9107. Reduce the granularity of Container locks for BlockDeletingService (apache#5149)
  HDDS-9270. Create a script to list all acceptance test splits (apache#5281)
  HDDS-9220. Let ContainerBalancerConfiguration#toString print more info (apache#5228)
  HDDS-9208. Add queue limit in ReplicationServer. (apache#5216)
  HDDS-9268. [Snapshot] Update list of snapshot apis to include lsDiff details in docs. (apache#5278)
  HDDS-9234. OM should shutdown immediately if certificate durations are invalid (apache#5243)
  HDDS-9136. Throw exception when rename fails during moveToTrash. (apache#5253)
  ...
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.

6 participants

Comments