Merged
Conversation
|
This pull request has been linked to Shortcut Story #23862: Don't retry REST request on curl error. |
ihnorton
reviewed
Dec 2, 2022
| *curl_code = tmp_curl_code; | ||
| } | ||
|
|
||
| /* If there is a write error we should not attempt a retry */ |
Member
There was a problem hiding this comment.
- IMO we should combine with the if block above for clarity
- IIUC the only condition leading to a retry now will be an HTTP status in one of the
retry_http_codes_. Is that correct? If so I think we should add a comment to clarify.
If we received a curl error we should not resubmit the original request. There are some curl errors that can occur after we have received partial results and we might have already deserialized and copied data into the user buffer. Retries resubmit the initial request not accounting for any partial state.
Shelnutt2
force-pushed
the
sethshelnutt/sc-23862/don-t-retry-rest-request-on-curl-error
branch
from
d577512 to
7c78be6
Compare
ihnorton
approved these changes
Dec 5, 2022
ihnorton
deleted the
sethshelnutt/sc-23862/don-t-retry-rest-request-on-curl-error
branch
github-actions Bot
pushed a commit
that referenced
this pull request
Dec 7, 2022
If we received a curl error we should not resubmit the original request. There are some curl errors that can occur after we have received partial results and we might have already deserialized and copied data into the user buffer. Retries resubmit the initial request not accounting for any partial state. --- TYPE: IMPROVEMENT DESC: Do not resubmit http requests for curl errors for REST requests
ihnorton
pushed a commit
that referenced
this pull request
Dec 8, 2022
If we received a curl error we should not resubmit the original request. There are some curl errors that can occur after we have received partial results and we might have already deserialized and copied data into the user buffer. Retries resubmit the initial request not accounting for any partial state. --- TYPE: IMPROVEMENT DESC: Do not resubmit http requests for curl errors for REST requests Co-authored-by: Seth Shelnutt <[email protected]>
KiterLuc
pushed a commit
that referenced
this pull request
Sep 4, 2024
Core-side retries on curl errors have been explicitly disallowed in the past in #3712 to protect from double writes in case of errors in a connection teardown phase, so after a fragment has been written, where a retry would just re-write the same fragment. However, we have experienced spurious "Connection reset by peer" type of errors especially during ingestion (Write queries), which we think could be mitigating by allowing retries for most of curl errors. (exclude those that typically happen during connection teardown). [[sc-54377](https://app.shortcut.com/tiledb-inc/story/54277/enable-retries-for-most-curl-error-codes-in-core)] --- TYPE: IMPROVEMENT DESC: Enable curl error retries. --------- Co-authored-by: Shaun M Reed <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If we received a curl error we should not resubmit the original request. There are some curl errors that can occur after we have received partial results and we might have already deserialized and copied data into the user buffer. Retries resubmit the initial request not accounting for any partial state.
TYPE: IMPROVEMENT
DESC: Do not resubmit http requests for curl errors for REST requests