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

Messages lost after unsubscribe before receipt with Auto ACK #135

Open
digilist opened this issue Oct 22, 2019 · 2 comments
Open

Messages lost after unsubscribe before receipt with Auto ACK #135

digilist opened this issue Oct 22, 2019 · 2 comments
Assignees
Labels

Comments

@digilist
Copy link

Calling unsubscribe() on the ConsumerState sends an unsubscribe frame to sever and waits for the receipt. While the client waits for the receipt, it is receiving further frames and writes them into the unprocessedFrames. After the receipt was received, the unsubscribe method replaces the state with a ProducerState leaving the application incapable of receiving the intermediate frames (between unsubscribe and receipt). (As the read method on the ProducerState throws an InvalidState exception).

Our solution was to read frames directly from the Client, but that's not really nice and took us a lot of debugging to find the issue for lost messages 😄

@jmglsn jmglsn self-assigned this Oct 22, 2019
@jmglsn
Copy link
Member

jmglsn commented Oct 22, 2019

I like your idea regarding a message draining as callback. I'll try to add test cases first.

@xyNNN
Copy link

xyNNN commented Nov 20, 2019

Haken dran.

jmglsn added a commit that referenced this issue Nov 30, 2019
When clients use `auto ack` and `unsubscribe` it's possible that messages are considered as processed, while they have never been forwarded by `read`.
In order to avoid accidental message los a DrainingConsumer state was added, that comes together with the `DrainingMessageException`.
Access to `Parser` internal buffer has been simplified, by adding `isBufferEmpty`.
Same applies for `Client` here `isBufferEmpty` and `flushBufferedFrames` were added.
jmglsn added a commit that referenced this issue Nov 30, 2019
When clients use `auto ack` and `unsubscribe` it's possible that messages are considered as processed, while they have never been forwarded by `read`.
In order to avoid accidental message los a DrainingConsumer state was added, that comes together with the `DrainingMessageException`.
Access to `Parser` internal buffer has been simplified, by adding `isBufferEmpty`.
Same applies for `Client` here `isBufferEmpty` and `flushBufferedFrames` were added.
jmglsn added a commit that referenced this issue Nov 30, 2019
When clients use `auto ack` and `unsubscribe` it's possible that messages are considered as processed, while they have never been forwarded by `read`.
In order to avoid accidental message los a DrainingConsumer state was added, that comes together with the `DrainingMessageException`.
Access to `Parser` internal buffer has been simplified, by adding `isBufferEmpty`.
Same applies for `Client` here `isBufferEmpty` and `flushBufferedFrames` were added.
jmglsn added a commit that referenced this issue Jun 15, 2020
When clients use `auto ack` and `unsubscribe` it's possible that messages are considered as processed, while they have never been forwarded by `read`.
In order to avoid accidental message los a DrainingConsumer state was added, that comes together with the `DrainingMessageException`.
Access to `Parser` internal buffer has been simplified, by adding `isBufferEmpty`.
Same applies for `Client` here `isBufferEmpty` and `flushBufferedFrames` were added.
jmglsn added a commit that referenced this issue Jun 15, 2020
When clients use `auto ack` and `unsubscribe` it's possible that messages are considered as processed, while they have never been forwarded by `read`.
In order to avoid accidental message los a DrainingConsumer state was added, that comes together with the `DrainingMessageException`.
Access to `Parser` internal buffer has been simplified, by adding `isBufferEmpty`.
Same applies for `Client` here `isBufferEmpty` and `flushBufferedFrames` were added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants