-
Notifications
You must be signed in to change notification settings - Fork 35
(#211) Ensure direct publishers respect context cancellation #2260
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
base: main
Are you sure you want to change the base?
Conversation
vjanelle
commented
Aug 6, 2025
- Add context cancellation checks to core publishing methods (PublishRaw, PublishRawMsg) in choria/connection.go.
- Update federation egest, stream adapter, and submission spool to check context before publishing.
- Improve client publisher to check context before publishing and connecting.
- Add context cancellation checks to core publishing methods (PublishRaw, PublishRawMsg) in choria/connection.go. - Update federation egest, stream adapter, and submission spool to check context before publishing. - Improve client publisher to check context before publishing and connecting.
ugh >.> |
broker/adapter/streams/output.go
Outdated
| defer obs.ObserveDuration() | ||
| defer func() { workqlen.Set(float64(len(sc.work))) }() | ||
|
|
||
| select { |
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 think all these can be just checking if ctx.Error is nil?
hard to say if this will really help unless the nats.go calls took contexts
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.
Less code, same effect - non-blocking.
And yeah, probably not immediately useful.
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.
@ripienaar the nats client takes custom dialers so we could use net.DialContext?
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.
That would only be for making the initial connection I think?
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.
Publishing tends to happen into a buffer in the client it wouldn’t block. The only really blocking thing is nc.Request