Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reset offsets in buffer list for retries #5220

Merged
merged 1 commit into from
Aug 2, 2024
Merged

Conversation

ypatia
Copy link
Member

@ypatia ypatia commented Jul 30, 2024

There's a bug in the retry logic when a 503 is encountered while uploading data. Our curl client fails to reset its BufferList state back to zero in this case and we end up sending a bunch of garbage data to the cloud service.

This PR fixes the issue and adds a regression test that failed before and passes with the fix.

[sc-49128]


TYPE: BUG
DESC: Reset offsets in buffer list for retries

@ypatia ypatia force-pushed the yt/sc-49128/fix_curl_retries branch 2 times, most recently from 9cc3684 to b647566 Compare July 30, 2024 12:32
@ypatia ypatia requested a review from shaunrd0 July 30, 2024 12:32
@ypatia ypatia force-pushed the yt/sc-49128/fix_curl_retries branch from b647566 to f35be0b Compare July 30, 2024 12:34
@Shelnutt2
Copy link
Member

Is there any changes needed related to buffer_list_seek_callback or CURLOPT_SEEKFUNCTION? We use this for the redirection which I believe is working fine but since it involves a similar type of reply of the buffers wanted to ask.

@ypatia
Copy link
Member Author

ypatia commented Jul 31, 2024

Is there any changes needed related to buffer_list_seek_callback or CURLOPT_SEEKFUNCTION? We use this for the redirection which I believe is working fine but since it involves a similar type of reply of the buffers wanted to ask.

Thanks for bringing this to my attention, I think this change is generic and applies to any retry with data stored in a BufferList structure, so it'll work for seek same as it works for plain read memory cb.

@ypatia ypatia merged commit 2a30fbe into dev Aug 2, 2024
61 checks passed
@ypatia ypatia deleted the yt/sc-49128/fix_curl_retries branch August 2, 2024 07:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants