feat(thrift): add fallback address option for shmipc #639
Conversation
| self.default_mkt.make_transport(addr).await | ||
| } | ||
|
|
||
| fn set_connect_timeout(&mut self, timeout: Option<std::time::Duration>) { |
There was a problem hiding this comment.
shall we set the timeouts to shmipc_mkt too?
volo/src/net/shmipc_fallback.rs
Outdated
| let shmipc_conn = self.shmipc_listener.accept().fuse(); | ||
| let default_conn = self.default_incoming.accept().fuse(); | ||
| futures::pin_mut!(shmipc_conn, default_conn); | ||
| match futures::future::select(shmipc_conn, default_conn).await { |
There was a problem hiding this comment.
When futures::future::select resolves, the other future is dropped. If the losing future was partway through accepting a connection, that connection may be lost.
Consider whether a tokio::select! with biased and cancellation-safe futures would be more appropriate, or at least document this trade-off?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #639 +/- ##
==========================================
- Coverage 43.77% 43.76% -0.02%
==========================================
Files 161 161
Lines 19662 19662
==========================================
- Hits 8608 8605 -3
- Misses 11054 11057 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
volo-thrift/src/client/mod.rs
Outdated
| pub fn address_with_fallback<A1: Into<Address>, A2: Into<Address>>( | ||
| self, | ||
| shmipc_addr: A1, | ||
| fallback_addr: A2, |
There was a problem hiding this comment.
Is is better to only pass fallback_addr to the method? As a result, the client build code looks like:
volo_gen::thrift_gen::hello::HelloServiceClientBuilder::new("psm")
.address()
.shmipc_fallback_address()
.build()
Joshuahoky
force-pushed
the
feat/shmipc_fallback
branch
from
a3dfde5 to
84d2d9d
Compare
3 participants
Motivation
Currently, volo-thrift supports shmipc transport for high-performance local communication but it may not be available or fail to connect. This PR introduces functionality which allows users to set a fallback address.
Solution
Added ShmipcMakeTransportWithFallback which wraps a DefaultMakeTransport