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

backend: list message concurrent stream send fix #1076

Merged
merged 5 commits into from
Feb 5, 2024

Conversation

bojand
Copy link
Member

@bojand bojand commented Feb 5, 2024

Multiple goroutines may perform writes (stream Send()s) to the stream via stream reporter. In stream reporter ensure these are guarded using a mutex.

Minor frontend cleanup to only have a single Console service client.

Fixes #1073.

@bojand bojand requested review from weeco and rikimaru0345 February 5, 2024 19:19
timeout = 31 * time.Minute
}

ctx, cancel := context.WithTimeoutCause(ctx, timeout, errors.New("list fetch timeout"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Very good to add the reason there 👍

frontend/src/state/backendApi.ts Outdated Show resolved Hide resolved
frontend/src/state/backendApi.ts Outdated Show resolved Hide resolved
Comment on lines +105 to +107
if message == nil {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering why this is necessary? When would we send nil here?

Copy link
Member Author

Choose a reason for hiding this comment

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

it was just a guard I added as I was debugging. we really shouldn't but might be a good idea.

@weeco weeco self-requested a review February 5, 2024 20:00
@bojand bojand merged commit 8a44126 into master Feb 5, 2024
9 checks passed
@bojand bojand deleted the backend/list_message_concurrent_stream_send_fix branch February 5, 2024 20:02
@bojand bojand restored the backend/list_message_concurrent_stream_send_fix branch April 12, 2024 00:25
@bojand bojand deleted the backend/list_message_concurrent_stream_send_fix branch April 12, 2024 00:40
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.

Inconsitent filtering messages on a topic
2 participants