command: retry upload on s3.NoSuchUpload #470
Conversation
With this change, doUpload method will retry the multipart upload on NoSuchUploadError if the "retry-on-no-such-upload" flag is used and the remote file has not been modified after the beginning of the execution (or does not exists at all) . Otherwise (if it has been modified) it will assume that upload was successfull, and ignore the error. Beware that, this workaround approach has some drawbacks to consider: 1) If upload really failed but a third person succesfuly modified the remote file (dsturl), then this code will incorrectly return "success". 2) It would be better to use checksum rather than the last modification date but, unfortunately it is not supported in aws-sdk-go v1.*, and it is a bit tricky to use ETag's as a replacement of checksum for the multipart uploads. Refer https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html#large-object-checksums see also the following as an example of using ETag as checksum https://github.com/peak/s3hash Updates #450
|
A few updates: On the other hand if we leave the retry logic in its current place ((Copy).doUpload) then it would be too high level to manipulate the mockAPI handlers/response to simulate the error for tests. *Even better simply add another testcase to storage.TestS3Retry. Update : |
kucukaslan
changed the title
Write simple test. The tests does not check if the logic works as expected for when the varios last modification dates of remote. All the test assumes that there is no (such) object in remote (since the response is cleared & replaced with NoSuchUpload error).
kucukaslan
requested review from
aykutfarsak and
seruman
and removed request for
a team
kucukaslan
changed the title
|
Yet another update: I've settled to place the retry logic into ps. I. updated the first comment accordingly and deleted the discussion about using modification date. You can use edit history in case you need them. |
Remove unused "StartTime" variable. Change flag name to "no-such-upload-retry-count" from "retry-on-no-such-upload". Also change corresponding fields' name to [N]oSuchUploadRetryCount. Move code generation to an external method.
... even though it is a hidden flag.
It will use debug level to log retry attemp. Debug level is also the level. that s3.customRetryer write logs, see also https://github.com/peak/s5cmd/blob/822043839c0e581f9b481c9ef30dd88a04463a09/storage/s3.go#L932-L934
|
I've tested the read-after-write consistency of object metadata in another branch. The storage.Stat request (after upload) operation for all of over 1000 objects returned the updated metadata. |
With this change, doUpload method will retry the multipart upload on NoSuchUploadError if the
no-such-upload-retry-countflag is used withcporsyncand the value of the special metadata in remote file does not match the one placed in upload request (or does not exists at all) . Otherwise (if it is different), it will assume that upload was successful and ignore the error.Retry logic is placed into
s3.Putsince it is possible to restart upload operation and write unit test there.For the retry condition, it will use a user defined metadata field. If the
no-such-upload-retry-countgiven (with positive parameter), then it will put a random string as a user defined metadata. Then, upon thes3.NoSuchUploaderror, it will check if that field matches with the one placed or not. If it does not match (or does not even exists), then the upload will be retried.Fixes #450