for downloading object, be pessimistic and check how many bytes got c… #3790
Conversation
| @@ -194,9 +194,17 @@ func DownloadFile(ctx context.Context, logger log.Logger, bkt BucketReader, src, | |||
| }() | |||
| defer runutil.CloseWithLogOnErr(logger, f, "download block's output file") | |||
|
|
|||
| if _, err = io.Copy(f, rc); err != nil { | |||
| atts, err := bkt.Attributes(ctx, src) | |||
There was a problem hiding this comment.
This will double the number of API calls required to download a file. I don't think we should do it.
There was a problem hiding this comment.
if don't do it. sometimes return value of io.Copy was less than the legnth of on the object storage.
the file was not complete, but was not err be return.
so ,the thanos compact will change state to [halt]. halt meas ....
if do it, thanos compact will retry.
i don't know some graceful approach to fix this issue.
There was a problem hiding this comment.
if don't do it. sometimes return value of io.Copy was less than the legnth of on the object storage.
Looks an issue with a specific backend storage that shouldn't be fixed here. Which backend storage are you using?
There was a problem hiding this comment.
I confirm I believe this is not the right place to address it, for a couple of reasons:
- We shouldn't introduce additional API calls
- If the reader returned by S3
Get()can return partial response without erroring out, then the problem is widespread across the whole codebase (any usage ofGet()) so fixing it here would just cover a small amount of use cases
Checking the logic to have a better proposal
There was a problem hiding this comment.
See: #2805 (comment)
Let's continue the discussion on the issue.
…opied Signed-off-by: timmy <[email protected]>
su47flying
force-pushed
the
check-download-what-was-copied
branch
from
8932e32 to
3a19a59
Compare
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
i got the same issue with [#2805]. Compact
i find some download object's length was not same with it's real length(on object storage).
this PR will check the copy lenght, if got the length was not equal to the real length, thanos will do retry.
Changes
Verification
in my env, after we apply this code, it works well.
i can get some retry log for this code.