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

COS: Fix tools replicate #5166

Merged
merged 3 commits into from
Feb 25, 2022
Merged

Conversation

hanjm
Copy link
Member

@hanjm hanjm commented Feb 17, 2022

Fix issue reported by @wu3396 (#5139 (comment) ), pr #5139 introduce multi-part upload, it uses TryToGetSize to get the size of reader, but it will error when the reader is wrapped by TimingReadCloser. this pr continue to fix it.

  1. Fix TryToGetSize error when the reader wrappered by TimingReadCloser, It will also fix cos/s3/bos/swift get size error when use objstore.TryToGetSize(r).
  2. Add size info into reader returned by (*bucket).Get for pass it to (*bucket).Upload function.

Signed-off-by: Jimmie Han hanjinming@outlook.com

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  1. Implement ObjectSizer interface for timingReadCloser.
  2. Add unit test.

Verification

  1. unit test.
  2. use thanos tool replicate to reproduce the issue and verify.

@hanjm hanjm changed the title objstore: fix TryToGetSize err when the reader wrappered by TimingReadCloser COS: fix TryToGetSize err when the reader wrappered by TimingReadCloser Feb 17, 2022
@hanjm hanjm changed the title COS: fix TryToGetSize err when the reader wrappered by TimingReadCloser COS: fix tools replicate Feb 17, 2022
@hanjm
Copy link
Member Author

hanjm commented Feb 18, 2022

docs check fail, it will fixed by #5167 (:

@hanjm hanjm changed the title COS: fix tools replicate COS: Fix tools replicate Feb 18, 2022
@yeya24
Copy link
Contributor

yeya24 commented Feb 21, 2022

Can you please rebase main? The doc should be fixed already on main.

yeya24
yeya24 previously approved these changes Feb 21, 2022
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

@hanjm hanjm force-pushed the fix/jimmiehan-fix-objsize branch 2 times, most recently from a2d07a2 to 6d55947 Compare February 22, 2022 05:30
@hanjm
Copy link
Member Author

hanjm commented Feb 22, 2022

Can you please rebase main? The doc should be fixed already on main.

Thank you reviewing. 🚀 Rebase Done.

yeya24
yeya24 previously approved these changes Feb 22, 2022
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM.

…dCloser

Signed-off-by: Jimmie Han <hanjinming@outlook.com>
Signed-off-by: Jimmie Han <hanjinming@outlook.com>
@hanjm
Copy link
Member Author

hanjm commented Feb 24, 2022

@yeya24 some e2e test is flaky, I retry it. Please review again, thank you.

@hanjm hanjm mentioned this pull request Feb 25, 2022
matej-g
matej-g previously approved these changes Feb 25, 2022
Copy link
Collaborator

@matej-g matej-g left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

pkg/objstore/cos/cos.go Outdated Show resolved Hide resolved
Co-authored-by: Matej Gera <38492574+matej-g@users.noreply.github.com>
Signed-off-by: Jimmie Han <hanjinming@outlook.com>
@GiedriusS GiedriusS merged commit 7528375 into thanos-io:main Feb 25, 2022
Nicholaswang pushed a commit to Nicholaswang/thanos that referenced this pull request Mar 6, 2022
* objstore: fix TryToGetSize err when the reader wrappered by TimingReadCloser

Signed-off-by: Jimmie Han <hanjinming@outlook.com>

* COS: Add size info into reader for pass it to Upload function.

Signed-off-by: Jimmie Han <hanjinming@outlook.com>

* Fix typo

Co-authored-by: Matej Gera <38492574+matej-g@users.noreply.github.com>
Signed-off-by: Jimmie Han <hanjinming@outlook.com>

Co-authored-by: Matej Gera <38492574+matej-g@users.noreply.github.com>
Signed-off-by: Nicholaswang <wzhever@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants