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

Re-reaise IncompleteReadError as MaybeRecoverableError #194

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

orange-kao
Copy link
Contributor

About this change - What it does

S3 and Azure can throw IncompleteReadError when the TCP session was disrupted. However, current implementation does not re-reaise it as MaybeRecoverableError. As a result, programs like pghoard will give up basebackup download.

This is more likely to affect large (2~4 TiB) databases, because large database takes longer to download base backup and more likely to encounter at least one IncompleteReadError.

This commit re-reaise IncompleteReadError as MaybeRecoverableError, so pghoard can retry on an object without starting over.

Why this way

I learned that pghoard use MaybeRecoverableError to decide whether it will retry or not. So I thought re-reaise IncompleteReadError as MaybeRecoverableError might be an option.

S3 and Azure can throw IncompleteReadError when the TCP session was
disrupted. However, current implementation does not re-reaise it as
MaybeRecoverableError. As a result, programs like pghoard will give up
basebackup download.

This is more likely to affect large (2~4 TiB) databases, because large
database takes longer to download base backup and more likely to
encounter at least one IncompleteReadError.

This commit re-reaise IncompleteReadError as MaybeRecoverableError, so
pghoard can retry on an object without starting over.
@orange-kao orange-kao force-pushed the orange-incomplete-read-error branch from cbb8dcf to 95c329e Compare October 28, 2024 22:46
@orange-kao
Copy link
Contributor Author

I have force-pushed the branch after fixing for make lint.

Copy link
Contributor

@aris-aiven aris-aiven left a comment

Choose a reason for hiding this comment

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

👍

@aris-aiven aris-aiven merged commit 7ed9186 into Aiven-Open:main Oct 29, 2024
9 checks passed
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