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

[FIXED] Subscribe timeout should send a close request #347

Merged
merged 1 commit into from
Feb 25, 2021

Conversation

kozlovic
Copy link
Member

If an application gets a timeout on a Subscribe() call, it does not
know if the server is actually going to process the request. The request
could have been just processed but the client timed-out before getting
the response back, or there is a bit of a backlog of requests in the
server and the request will be processed after the request times out
in the client.

If that happens, and if the application maintains its connection, then
it is possible that the server processes the subscription and keep that
subscription alive, although the application does not have an handle
on this subscription (the Subscribe() call has failed).

On timeout, the library will now send a request to close the subscription.
However, the protocol normally requires the AckInbox which is assigned by
the server and sent back as part of the subscription response protocol.
The library will send the request with the subscription inbox. Newer
servers (v0.21.0+) will be able to handle that, older will report that no
subscription was found for the subscription close request.

Signed-off-by: Ivan Kozlovic ivan@synadia.com

If an application gets a timeout on a Subscribe() call, it does not
know if the server is actually going to process the request. The request
could have been just processed but the client timed-out before getting
the response back, or there is a bit of a backlog of requests in the
server and the request will be processed after the request times out
in the client.

If that happens, and if the application maintains its connection, then
it is possible that the server processes the subscription and keep that
subscription alive, although the application does not have an handle
on this subscription (the Subscribe() call has failed).

On timeout, the library will now send a request to close the subscription.
However, the protocol normally requires the AckInbox which is assigned by
the server and sent back as part of the subscription response protocol.
The library will send the request with the subscription inbox. Newer
servers (v0.21.0+) will be able to handle that, older will report that no
subscription was found for the subscription close request.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 92.996% when pulling 6e44911 on fix_sub_req_timeout_handling into cebf22c on master.

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants