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

Use correct target path during mirroring objects #4807

Merged
merged 2 commits into from
Dec 29, 2023

Conversation

shtripat
Copy link
Contributor

Description

Set correct target path for mirrored objects

Motivation and Context

Issue: #4806

Currently if we don't use absolute paths in mirror command, absolute path of source gets appended to the target path and so in destination object gets mirrored to wrong prefix / path.

For example say source object path is /tmp/source/prefix/a.txt and mirror command executed from /tmp dir as mc mirror --json --watch source xyz, the object in target gets mirrored to location xyz/tmp/source/prefix/a.txt.

mc mirror --json --watch source/ xyz/
{
 "status": "success",
 "source": "/tmp/source/prefix/a.txt",
 "target": "xyz/tmp/source/prefix/a.txt",
 "size": 0,
 "totalCount": 1,
 "totalSize": 0
}
{
 "status": "success",
 "source": "/tmp/source/b.txt",
 "target": "xyz/b.txt",
 "size": 0,
 "totalCount": 2,
 "totalSize": 0
}

With the fix now after running the command mc mirror --json --watch source xyz the object would correctly get copied to location /tmp/xyz/prefix/a.txt as below

{
 "status": "success",
 "source": "/tmp/source/prefix/a.txt",
 "target": "xyz/prefix/a.txt",
 "size": 0,
 "totalCount": 2,
 "totalSize": 0
}
{
 "status": "success",
 "source": "/tmp/source/b.txt",
 "target": "xyz/b.txt",
 "size": 0,
 "totalCount": 1,
 "totalSize": 0
}
{
 "status": "success",
 "source": "/tmp/source/c.txt",
 "target": "xyz/c.txt",
 "size": 0,
 "totalCount": 3,
 "totalSize": 0
}

Community Contribution License

All community contributions in this pull request are licensed to the project maintainers
under the terms of the [Apache 2 license] (https://www.apache.org/licenses/LICENSE-2.0).
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.

How to test this PR?

Refer issue# #4806
Usual mirror with absolute paths should work as is.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

Currently if we dont use absolute paths in mirror command, absolute path of
source gets appended to the target path and so in destination object gets
mirrored to wrong prefix / path.

For example say source object path is `/tmp/source/prefix/a.txt` and mirror
command executed from `/tmp` dir as `mc mirror --json --watch source xyz`,
the object in target gets mirrored to location `xyz/tmp/source/prefix/a.txt`.
Also any new file created gets copied properly though.

```
mc mirror --json --watch source/ xyz/
{
 "status": "success",
 "source": "/tmp/source/prefix/a.txt",
 "target": "xyz/tmp/source/prefix/a.txt",
 "size": 0,
 "totalCount": 1,
 "totalSize": 0
}
{
 "status": "success",
 "source": "/tmp/source/b.txt",
 "target": "xyz/b.txt",
 "size": 0,
 "totalCount": 2,
 "totalSize": 0
}

```

With the fix now after running the command `mc mirror --json --watch source xyz`
the object would correctly get copied to location `/tmp/xyz/prefix/a.txt` as below

```
{
 "status": "success",
 "source": "/tmp/source/prefix/a.txt",
 "target": "xyz/prefix/a.txt",
 "size": 0,
 "totalCount": 2,
 "totalSize": 0
}
{
 "status": "success",
 "source": "/tmp/source/b.txt",
 "target": "xyz/b.txt",
 "size": 0,
 "totalCount": 1,
 "totalSize": 0
}
{
 "status": "success",
 "source": "/tmp/source/c.txt",
 "target": "xyz/c.txt",
 "size": 0,
 "totalCount": 3,
 "totalSize": 0
}
```

Signed-off-by: Shubhendu Ram Tripathi <[email protected]>
@harshavardhana
Copy link
Member

This seems to be only for filesystem to filesystem? @shtripat

@vadmeste
Copy link
Member

Let me take a through look at this; I would say most likley there is a regression somewhere that caused this.

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

One comment

@shtripat
Copy link
Contributor Author

This seems to be only for filesystem to filesystem? @shtripat

Yes

Signed-off-by: Shubhendu Ram Tripathi <[email protected]>
@shtripat shtripat requested a review from vadmeste December 29, 2023 04:07
Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

LGTM

@harshavardhana harshavardhana merged commit 5386533 into minio:master Dec 29, 2023
5 checks passed
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