-
Notifications
You must be signed in to change notification settings - Fork 664
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
[FIXED] Cleanup reply sub after putting object #1282
Conversation
Signed-off-by: Piotr Piotrowski <piotr@synadia.com>
if err != nil { | ||
return nil, err | ||
} | ||
|
||
defer jetStream.(*js).cleanupReplySub() |
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.
qq, why do we have to type assert here?
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.
nc.JetStream()
returns an interface, not *js
so I could either add cleanupReplySub()
as interface method or type assert here - I decided not to clutter the interface with another method (even unexported), but not sure which approach is better.
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.
ah right... makes sense
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
likely unrelated flapper but might be worth looking at:
|
@wallyqs yeah, from what I saw that's a flapper and it's on the list :) |
Async reply subscription was created on each
object.Put()
and not cleaned up. This PR closes this subscription afterPut()
completes.