-
Notifications
You must be signed in to change notification settings - Fork 262
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(base): Increase the room_info_notable_update_sender
capacity
#4013
feat(base): Increase the room_info_notable_update_sender
capacity
#4013
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.
Nice catch! LGTM except for the typo 😉 .
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.
Thanks for adding logs too.
(Should we remove the code that's commented out? Would be fairly easy to add it back later anyways.)
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.
😝
fe00c61
to
d3feae7
Compare
I've removed the comments as suggested by @bnjbvr and I've used |
Err(RecvError::Closed) => { | ||
error!("Cannot receive room info notable updates because the sender has been closed"); | ||
} |
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.
hacktualley: if this stream has closed, then this infinite loop will iloop on this case, right? Maybe we should return at this point?
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'm adding a little break
then. Good catch. It would have been caught previously, but not anymore.
This patch updates `merge_stream_and_receiver` to display an `error!` when the room info receiver reads an error, like `Closed` or `Lagged`. This is helpful when debugging.
This broadcast channel can easily be overflowed if more than 100 updates arrive at the time. This patch extends the capacity to 2^16 - 1.
d3feae7
to
f00bbf8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4013 +/- ##
==========================================
- Coverage 84.27% 84.24% -0.04%
==========================================
Files 266 266
Lines 28336 28343 +7
==========================================
- Hits 23880 23877 -3
- Misses 4456 4466 +10 ☔ View full report in Codecov by Sentry. |
This broadcast channel can easily be overflowed if more than 100 updates arrive
at the time. This patch extends the capacity to 2^16 - 1.
This patch also adds more logs to detect lagged on this channel from the room
list point of view.