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

fix: Implement timeout layer correctly by using timeout #4059

Merged
merged 10 commits into from
Jan 24, 2024
Merged

Conversation

Xuanwo
Copy link
Member

@Xuanwo Xuanwo commented Jan 23, 2024

TimeoutLayer is using tokio::time::timeout to implement timeout for operations. And IO Operations insides reader, writer will use Pin<Box<tokio::time::Sleep>> to track the timeout.

This might introduce a bit overhead for IO operations, but it's the only way to implement timeout correctly. We used to implement timeout layer in zero cost way that only stores a std::time::Instant and check the timeout by comparing the instant with current time. However, it doesn't works for all cases.

For examples, users TCP connection could be in Busy ESTAB state. In this state, no IO event will be emit. The runtime will never poll our future again. From the application side, this future is hanging forever until this TCP connection is closed for reaching the linux net.ipv4.tcp_retries2 times.

Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Copy link
Member

@PsiACE PsiACE left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks for sharing the blog and the detailed explanation in the code.

@Xuanwo Xuanwo merged commit 5dc2c7e into main Jan 24, 2024
227 checks passed
@Xuanwo Xuanwo deleted the fix-timeout-layer branch January 24, 2024 02:46
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.

2 participants