Skip to content

HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.#3906

Merged
neils-dev merged 5 commits intoapache:masterfrom
sadanand48:HDDS-5866
Nov 3, 2022
Merged

HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs.#3906
neils-dev merged 5 commits intoapache:masterfrom
sadanand48:HDDS-5866

Conversation

@sadanand48
Copy link
Contributor

What changes were proposed in this pull request?

When moving a file/dir to trash, the destination path is different for ofs and o3fs. The change here is to make it same for both.


o3fs -> /<vol>/<buck>/.Trash/<user>/Current/..<dir if any>..
ofs -> /<vol>/<buck>/.Trash/<user>/Current/<vol>/<buck>/..<dir if any>..

Omitting volume and bucket for ofs key since its redundant info as for a particular ozone key already volume and bucket is known.

Most of the code for moveToTrash is duplicated from the hadoop client code and only the below if condition is a modification to the method.
If this part is not duplicated and the path is trimmed of volume and bucket name and passed to super.moveToTrash(path) it will fail as part of getFileStatus call as it would call getFileStatus(ofs://serviceId/key).

 if (fs.getUri().getScheme().equals(OzoneConsts.OZONE_OFS_URI_SCHEME)) {
        OFSPath ofsPath = new OFSPath(path);
        // trimming volume and bucket in order to be compatible with o3fs
        // Also including volume and bucket name in the path is redundant as
        // the key is already in a particular volume and bucket.
        Path trimmedVolumeAndBucket =
            new Path(OzoneConsts.OZONE_URI_DELIMITER
                + ofsPath.getKeyName());
        trashPath = makeTrashRelativePath(trashCurrent, trimmedVolumeAndBucket);
        baseTrashPath = makeTrashRelativePath(trashCurrent,
            trimmedVolumeAndBucket.getParent());
      } else {
        trashPath = makeTrashRelativePath(trashCurrent, path);
        baseTrashPath = makeTrashRelativePath(trashCurrent, path.getParent());
      }

What is the link to the Apache JIRA

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

How was this patch tested?

unit test

Copy link
Contributor Author

@sadanand48 sadanand48 Oct 28, 2022

Choose a reason for hiding this comment

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

This test is flaky as it checks for existence of key in /Current but the checkpoint interval is 1.5 seconds so it might already be in checkpoint directory. This part is already tested in testTrash , hence removing this test.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the info!

@sadanand48 sadanand48 changed the title HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs. HDDS-5866. Discrepancy in Trash directory in ofs vs o3fs. Oct 28, 2022
@sadanand48 sadanand48 marked this pull request as draft October 28, 2022 11:31
@sadanand48 sadanand48 marked this pull request as ready for review October 28, 2022 11:33
@neils-dev
Copy link
Contributor

+1
Thanks @sadanand48, re-run the workflow for the integration tests; looks like an unrelated error.

@neils-dev neils-dev merged commit 5dd389b into apache:master Nov 3, 2022
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