-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
adds keep-alive-interval to repair QUIC transport config #33866
Conversation
ea70d31
to
3b35ea0
Compare
Codecov Report
@@ Coverage Diff @@
## master #33866 +/- ##
=======================================
Coverage 81.9% 81.9%
=======================================
Files 811 811
Lines 219356 219359 +3
=======================================
+ Hits 179732 179735 +3
Misses 39624 39624 |
const MAX_CONCURRENT_BIDI_STREAMS: VarInt = VarInt::from_u32(512); | ||
const MAX_IDLE_TIMEOUT: Duration = Duration::from_secs(10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see definitions for these in sdk/src/quic.rs
. Can you add comments about why new definitions are needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ones in sdk are used for tpu, which has very different properties than repair. In particular tpu packets are only forwarded to the upcoming leaders which we know in advance who they are, it is a uni-directional traffic, and has a very different load and latency tolerance. Repair is bi-directional request-response, peers are randomly sampled, etc.
I pretty much rather not to reuse the same parameters at this stage until this code is stable and repair over quic protocol is fully rolled out over mainnet. Until then reusing same parameters can introduce regressions on one protocol while trying to optimize another protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just add some code comments on the reasoning will help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
let mut config = TransportConfig::default(); | ||
config | ||
.datagram_receive_buffer_size(None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment this to disable datagrams?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
3b35ea0
to
fe866f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
QUIC connections may timeout due to infrequent repair requests. The commit configures keep_alive_interval and max_idle_timeout to avoid timeouts. (cherry picked from commit 3ac2507)
…port of #33866) (#33992) adds keep-alive-interval to repair QUIC transport config (#33866) QUIC connections may timeout due to infrequent repair requests. The commit configures keep_alive_interval and max_idle_timeout to avoid timeouts. (cherry picked from commit 3ac2507) Co-authored-by: behzad nouri <[email protected]>
…port of solana-labs#33866) (solana-labs#33992) adds keep-alive-interval to repair QUIC transport config (solana-labs#33866) QUIC connections may timeout due to infrequent repair requests. The commit configures keep_alive_interval and max_idle_timeout to avoid timeouts. (cherry picked from commit 3ac2507) Co-authored-by: behzad nouri <[email protected]>
…port of solana-labs#33866) (solana-labs#33992) adds keep-alive-interval to repair QUIC transport config (solana-labs#33866) QUIC connections may timeout due to infrequent repair requests. The commit configures keep_alive_interval and max_idle_timeout to avoid timeouts. (cherry picked from commit 3ac2507) Co-authored-by: behzad nouri <[email protected]>
Problem
QUIC connections timeout due to infrequent repair requests.
Summary of Changes
Added
keep_alive_interval
to transport config.