common icon indicating copy to clipboard operation
common copied to clipboard

Should pkg/retry retry on HTTP status 500? (502? others?)

Open Windfarer opened this issue 2 years ago • 5 comments

when copy from remote repository, the current error retry does not handle status code 500 response. it seems there’s no handle for any error http status code in https://github.com/containers/common/blob/main/pkg/retry/retry.go maybe this error status code handler would be implemented in skopeo?

Windfarer avatar Dec 01 '23 03:12 Windfarer

Did you try using skopeo copy --retry-times N? It's set to 0 by default but you are free to set it to a value of your choice.

vrothberg avatar Dec 01 '23 09:12 vrothberg

yes, i've set this argument --retry-times 10, but I found that it only retries on EOF error, but when get a 500 status code, it will fail without any retry.

Valentin Rothberg @.***> 于2023年12月1日周五 17:09写道:

Did you try using skopeo copy --retry-times N? It's set to 0 by default but you are free to set it to a value of your choice.

— Reply to this email directly, view it on GitHub https://github.com/containers/common/issues/1759, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVVZWJIDI3O3AHNWJCBZ2DYHGNDDAVCNFSM6AAAAABACGWTMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZVG4ZTCMBUHE . You are receiving this because you authored the thread.Message ID: @.***>

-- farer.org

Windfarer avatar Dec 01 '23 12:12 Windfarer

Can you elaborate on the reasoning for retrying on 500? If a server returns 500, it indicates something's seriously wrong. I don't think it's conventional to assume the next response would be any different.

vrothberg avatar Dec 01 '23 15:12 vrothberg

For example, the source server may suffer from temporary remote storage unavaiable, return 500, or maybe temporary down or restart, return 502. Especially, we use skopeo copy in a daily rerun CI job. As a client we don't care about what happened to the server that we can't fix, and just want retry several times during any copy failure.

In other words, it would be better to retry unconditionally when the argument is set for handling any kind of failure. If the retry is conditional, the conditions which will be retry should be described in documents.

On Fri, Dec 1, 2023, 23:04 Valentin Rothberg @.***> wrote:

Can you elaborate on the reasoning for retrying on 500? If a server returns 500, it indicates something's seriously wrong. I don't think it's conventional to assume the next response would be any different.

— Reply to this email directly, view it on GitHub https://github.com/containers/common/issues/1759, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVVZWKAQQJOTP76DTV532DYHHWWVAVCNFSM6AAAAABACGWTMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZWGI3TAOJZGA . You are receiving this because you authored the thread.Message ID: @.***>

Windfarer avatar Dec 02 '23 01:12 Windfarer

There are quite a few situations, like running out of disk space, where retrying can make things worse. So I lean towards being conservative here, but I realize I tend to worry more than others.

Either way, this should be decided in c/common/pkg/retry (although it might need new error types exported from c/image as well, the heuristic needs to be decided in c/common), and Skopeo will not add a special case; transferring to that repo.

mtrmac avatar Dec 04 '23 14:12 mtrmac