-
Notifications
You must be signed in to change notification settings - Fork 173
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
feat(websocket client): WIP V2 ring buffer channel #165
Conversation
@@ -19,6 +19,7 @@ parking_lot = "0.11.1" | |||
pin-project = "1.0.1" | |||
jsonrpsee-proc-macros = { path = "proc-macros" } | |||
rand = "0.7.3" | |||
ring-channel = { git = "https://github.com/niklasad1/ring-channel.git", branch = "na-atomicptr" } |
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.
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.
Were you planning to open a PR for this?
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.
It's up to the author of the library, he doesn't seem to be very keen on using AtomicPtr
instead of NonNullPtr
let's see how it goes. However, the idea is to split this PR into two parts when it works properly
- Extract
try_send/send.now_or_never
into a separate PR and handle the errors withoutring channel crate
- Support for ring channel for ring channel.
src/client/ws/client.rs
Outdated
} | ||
|
||
impl<T> InternalChannelSender<T> { | ||
fn send(self, data: T) -> Result<(), Error> { |
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.
TODO: rename to send_non_blocking
and provide an additional method send_async
or something
I will split this into two different PRs instead |
Super far from being mergeable but it fixes the deadlock at least in #143
It makes is it user-configurable to allow losses for requests and subscriptions