Skip to content

Conversation

@shanielh
Copy link
Contributor

@shanielh shanielh commented Jun 9, 2024

This commit aims to fix issue #10340 where a connection reset is thrown from Apache HTTP Client (even when Keep Alive is set to true)

This commit aims to fix issue apache#10340 where a connection reset is thrown
from Apache HTTP Client (even when Keep Alive is set to true)
@github-actions github-actions bot added the AWS label Jun 9, 2024
@singhpk234
Copy link
Contributor

weren't we thinking of fixing this by retrys in the broader level #10433 ?

@shanielh
Copy link
Contributor Author

weren't we thinking of fixing this by retrys in the broader level #10433 ?

Haven't seen #10433 since it wasn't connected to the issue :( , note that this fix only fixes reading from a stream that was closed due to connection reset (which happens when you idle stream for a long period) and doesn't add retry mechanism over existing retry mechanisms of AWS SDK when calling SDK requests (I.e read tail and read specific byte range)

@amogh-jahagirdar
Copy link
Contributor

@shanielh Sorry about missing to link the issue on #10433, that's totally my bad. As @singhpk234 said, that PR should address the issue you're seeing here. I'll be revisiting the PR this week.

Although one interesting thing about this PR is that you're retrying SocketException, which I should also be retrying on my PR (currently retrying SocketTimeoutException for client side connection reset, but SocketException can occur and be retried for server side).

@shanielh
Copy link
Contributor Author

@shanielh Sorry about missing to link the issue on #10433, that's totally my bad. As @singhpk234 said, that PR should address the issue you're seeing here. I'll be revisiting the PR this week.

Although one interesting thing about this PR is that you're retrying SocketException, which I should also be retrying on my PR (currently retrying SocketTimeoutException for client side connection reset, but SocketException can occur and be retried for server side).

No worries, I see this PR is still on draft, any chances to look @ my PR as I validated and it fixes an issue in real world situation where I read a long partition file and taking a long sleep between row groups, before this PR, when getting to process the second row group I get "SocketException".

What is the expected time to merge #10433?

@github-actions
Copy link

github-actions bot commented Nov 7, 2024

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 7, 2024
@github-actions
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Nov 15, 2024
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.

3 participants