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

RetryIf might need more information and greater control over the execution flow. #1850

Merged
merged 1 commit into from
Sep 1, 2024

Conversation

newacorn
Copy link
Contributor

Another implementation of RetryIfErr

* Support for delayed retry logic
* Support for resetting the request timeout before retrying
* Users can decide whether to retry based on the request error and the number of retries.
Estimating the time required for a request to complete is relatively straightforward, but when retries are factored in, it becomes challenging to gauge this time accurately.

At the same time, comment out the logic for retrying non-idempotent requests:

	if !retry {
			break
			// Retry non-idempotent requests if the server closes
			// the connection before sending the response.
			//
			// This case is possible if the server closes the idle
			// keep-alive connection on timeout.
			//
			// Apache and nginx usually do this.
			//
			// This assertion cannot ensure that the server has not read our request.
			// Even if part of the request has been read, retrying for a non-idempotent
			// request is still unsafe. It is better to leave this decision to the user.
			//
			// if err != io.EOF {
			//	break
			// }
		}

This logic only covers one case where the error is not io.EOF; in fact, there are many other situations and edge cases. For non-idempotent requests, we should not make assumptions for the user.

@newacorn newacorn changed the title Fashthttp RetryIfErr Impl. RetryIf might need more information and greater control over the execution flow. Aug 27, 2024
client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
client.go Outdated Show resolved Hide resolved
* Support for delayed retry logic
* Support for resetting the request timeout before retrying
* Users can decide whether to retry based on the request error and the number of retries.
Estimating the time required for a request to complete is relatively straightforward, but when retries are factored in, it becomes challenging to gauge this time accurately.
@erikdubbelboer erikdubbelboer merged commit b33e869 into valyala:master Sep 1, 2024
15 checks passed
@erikdubbelboer
Copy link
Collaborator

Thanks!

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.

2 participants