Skip to content
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

chore: add metrics to redis collab stream #1116

Merged
merged 2 commits into from
Jan 3, 2025
Merged

Conversation

Horusiath
Copy link
Collaborator

@Horusiath Horusiath commented Jan 3, 2025

This PR comes with 2 changes:

  1. CollabStreamMetrics type which allows us to track how many streams are we scheduling to be read at given time:
  • collab_stream_reads_enqueued is incremented for every stream that has been observed. When the Redis xread returns and stream needs to be rescheduled for the next poll iteration, it also becomes enqueued.
  • collab_stream_reads_dequeued is incremented for every stream that scheduler returned for xread. These two metrics should be fairly close to each other. If they reads_enqueued grows faster than reads_dequeued it means, that we schedule new stream read requests faster than we can handle them.
  1. It fixes the issue when an observer was not correctly dropped and stayed inside of stream router for as long as no new objects where pulled from its corresponding stream.

for key in keys {
if let Some(last_id) = ids.next() {
if let Some((sender, _)) = senders.remove(key.as_str()) {
if sender.is_closed() {
Copy link
Collaborator Author

@Horusiath Horusiath Jan 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that we only unscheduled the stream subscriber if it was closed AND stream returned at least one message that we wanted to forward into (now closed) subscriber. If no new messages from stream were received, it will be rescheduled again even if subscriber was closed.

This could mean that if we had 5000 subscribers in the past, but no new data was coming from their Redis streams, and only 5 of them where active atm. we still tried to XREAD 5000 streams instead of 5.

@khorshuheng khorshuheng merged commit 424796e into main Jan 3, 2025
9 of 11 checks passed
@khorshuheng khorshuheng deleted the collab-stream-metrics branch January 3, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants