Skip to content

Enable curl error retries.#5273

Merged
KiterLuc merged 4 commits intodevfrom
yt/sc-54277/enable_curl_error_retries
Sep 4, 2024
Merged

Enable curl error retries.#5273
KiterLuc merged 4 commits intodevfrom
yt/sc-54277/enable_curl_error_retries

Conversation

@ypatia
Copy link
Copy Markdown
Member

@ypatia ypatia commented Sep 3, 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]


TYPE: IMPROVEMENT
DESC: Enable curl error retries.

@ypatia ypatia marked this pull request as ready for review September 3, 2024 14:04
@ypatia ypatia requested a review from a team as a code owner September 3, 2024 14:04
@ypatia
Copy link
Copy Markdown
Member Author

ypatia commented Sep 3, 2024

/backport to release-2.26

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 3, 2024

Copy link
Copy Markdown
Contributor

@nickvigilante nickvigilante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved tiledb/api/c_api/config/config_api_external.h and tiledb/sm/cpp_api/config.h only.

@ypatia ypatia force-pushed the yt/sc-54277/enable_curl_error_retries branch from 42a2335 to 25393b8 Compare September 3, 2024 14:34
Copy link
Copy Markdown
Member

@shaunrd0 shaunrd0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread tiledb/sm/rest/curl.cc Outdated
@KiterLuc KiterLuc changed the title Enable curl error retries Enable curl error retries. Sep 3, 2024
@ypatia ypatia force-pushed the yt/sc-54277/enable_curl_error_retries branch from 78ed593 to d43b024 Compare September 4, 2024 07:17
@ypatia ypatia force-pushed the yt/sc-54277/enable_curl_error_retries branch from d43b024 to 75a07f5 Compare September 4, 2024 07:22
@KiterLuc KiterLuc merged commit 0ac064b into dev Sep 4, 2024
@KiterLuc KiterLuc deleted the yt/sc-54277/enable_curl_error_retries branch September 4, 2024 08:49
KiterLuc pushed a commit that referenced this pull request Sep 4, 2024
Backport of #5273 to release-2.26

---
TYPE: IMPROVEMENT
DESC: Enable curl error retries.

---------

Co-authored-by: Ypatia Tsavliri <[email protected]>
Co-authored-by: Shaun M Reed <[email protected]>
@teo-tsirpanis
Copy link
Copy Markdown
Member

I think we should backpot this to 2.27 as well.

@KiterLuc
Copy link
Copy Markdown
Contributor

KiterLuc commented Sep 4, 2024

@teo-tsirpanis we might have to but we might get rid of the current release-2.27 branch so let's not do it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants