-
Notifications
You must be signed in to change notification settings - Fork 161
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
Doc: Add documentation for AsyncRuntime::Watch
traits
#1240
Doc: Add documentation for AsyncRuntime::Watch
traits
#1240
Conversation
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.
LGTM, just some nits:)
Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @drmingdrmer)
openraft/src/type_config/async_runtime/watch/mod.rs
line 12 at r1 (raw file):
/// A `Watch` is a pair of `WatchSender` and `WatchReceiver` that can be used to watch for changes /// to a value. /// A multi-producer, multi-consumer channel that only retains the last sent value.
Our WatchSender
trait does not have Clone
as its supertrait, so technically, it is not a multi-producer channel, though tokio::sync::watch::Sender
has Clone
implemented:>
openraft/src/type_config/async_runtime/watch/mod.rs
line 38 at r1 (raw file):
/// ```text /// // Task 1 (on thread A) | // Task 2 (on thread B) /// let _ref1 = rx.borrow(); |
- /// let _ref1 = rx.borrow(); |
+ /// let _ref1 = rx.borrow_watched(); |
In our impl, we call this method borrow_watched()
😀
openraft/src/type_config/async_runtime/watch/mod.rs
line 42 at r1 (raw file):
/// | let _ = tx.send(()); /// // may deadlock | /// let _ref2 = rx.borrow(); |
Same as above.
Add comprehensive documentation for the `Watch` trait and its associated traits in the `AsyncRuntime` module. This improves code clarity and helps developers understand the purpose and usage and behavior of these traits. - Fix: databendlabs#1238
6ba650d
to
2c4023d
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.
Thank you for your detailed review and suggestions!
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @SteveLauC)
openraft/src/type_config/async_runtime/watch/mod.rs
line 12 at r1 (raw file):
Previously, SteveLauC (SteveLauC) wrote…
Our
WatchSender
trait does not haveClone
as its supertrait, so technically, it is not a multi-producer channel, thoughtokio::sync::watch::Sender
hasClone
implemented:>
Done.
openraft/src/type_config/async_runtime/watch/mod.rs
line 38 at r1 (raw file):
Previously, SteveLauC (SteveLauC) wrote…
- /// let _ref1 = rx.borrow(); | + /// let _ref1 = rx.borrow_watched(); |In our impl, we call this method
borrow_watched()
😀
Done.
openraft/src/type_config/async_runtime/watch/mod.rs
line 42 at r1 (raw file):
Previously, SteveLauC (SteveLauC) wrote…
Same as above.
Done.
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.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions
Changelog
Doc: Add documentation for
AsyncRuntime::Watch
traitsAdd comprehensive documentation for the
Watch
trait and its associated traitsin the
AsyncRuntime
module. This improves code clarity and helps developersunderstand the purpose and usage and behavior of these traits.
Raft::metrics()
can block the RaftCore async task and underlying OS thread #1238This change is