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

Fix the batch message ack for WebSocket proxy. #12530

Merged
merged 3 commits into from
Oct 29, 2021

Conversation

codelipenghui
Copy link
Contributor

The WebSocket proxy uses the consumer instance to process the message acknowledgment.
But for the batch message acknowledgment, the batch message instance that deserialized from WebSocket proxy will with the same batch message acker, which will lead to the batch message can't be acked because the consumer only sends the ack request to the broker while the acker becomes empty.

The fix is to introduce a map to maintain an original which is used for getting batch message acker.
Added the test for covering the batch message ack.

The WebSocket proxy uses the consumer instance to process the message acknowledgement.
But for the batch message acknowledgement, the batch message instance that deserialized from WebSocket proxy will with the same batch message acker, which will lead to the batch message can't been acked because of the consumer only send the ack request to the broker while the acker becomes empty.

The fix is introduce a map to maintain an original which used for getting batch message acker.
Added the test for covering the batch message ack.
@codelipenghui codelipenghui added type/bug The PR fixed a bug or issue reported a bug area/websocket doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.1 labels Oct 29, 2021
@codelipenghui codelipenghui added this to the 2.10.0 milestone Oct 29, 2021
@codelipenghui codelipenghui self-assigned this Oct 29, 2021
@merlimat merlimat merged commit dfe0247 into apache:master Oct 29, 2021
merlimat added a commit that referenced this pull request Oct 29, 2021
* Fix the batch message ack for WebSocket proxy.

The WebSocket proxy uses the consumer instance to process the message acknowledgement.
But for the batch message acknowledgement, the batch message instance that deserialized from WebSocket proxy will with the same batch message acker, which will lead to the batch message can't been acked because of the consumer only send the ack request to the broker while the acker becomes empty.

The fix is introduce a map to maintain an original which used for getting batch message acker.
Added the test for covering the batch message ack.

* Simplified the logic a bit

* Using time-bound cleanup for ack-timeout cases

Co-authored-by: Matteo Merli <mmerli@apache.org>
merlimat added a commit that referenced this pull request Oct 29, 2021
* Fix the batch message ack for WebSocket proxy.

The WebSocket proxy uses the consumer instance to process the message acknowledgement.
But for the batch message acknowledgement, the batch message instance that deserialized from WebSocket proxy will with the same batch message acker, which will lead to the batch message can't been acked because of the consumer only send the ack request to the broker while the acker becomes empty.

The fix is introduce a map to maintain an original which used for getting batch message acker.
Added the test for covering the batch message ack.

* Simplified the logic a bit

* Using time-bound cleanup for ack-timeout cases

Co-authored-by: Matteo Merli <mmerli@apache.org>
@merlimat merlimat added cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life labels Oct 30, 2021
@codelipenghui codelipenghui deleted the penghui/fix-websocket-batch-ack branch November 1, 2021 01:26
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request Nov 29, 2021
* Fix the batch message ack for WebSocket proxy.

The WebSocket proxy uses the consumer instance to process the message acknowledgement.
But for the batch message acknowledgement, the batch message instance that deserialized from WebSocket proxy will with the same batch message acker, which will lead to the batch message can't been acked because of the consumer only send the ack request to the broker while the acker becomes empty.

The fix is introduce a map to maintain an original which used for getting batch message acker.
Added the test for covering the batch message ack.

* Simplified the logic a bit

* Using time-bound cleanup for ack-timeout cases

Co-authored-by: Matteo Merli <mmerli@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy area/websocket cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.0 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants