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

[Java Client] Avoid sending flow requests with zero permits #10507

Merged
merged 1 commit into from
May 7, 2021

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented May 7, 2021

Motivation

It's the Java client side fix for #10506.

When broker receives a FLOW request with zero permit, an IllegalArgumentException will be thrown in ServerCnx#handleFlow and then the connection will be closed so that the consumer will reconnect to the broker. If resume() was called for a zero queue consumer, the permits may be zero and trigger a reconnection. The frequent reconnections may cause messages repeated or out of order.

Modifications

  • Validate the permits of a FLOW command before sending it.
  • Add a test that a zero queue consumer resumes and pauses for each message periodically. Before this PR, there's a great chance that the received messages are repeated or out of order, even a ACK was sent.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests ZeroQueueSizeTest#testPauseAndResumeNoReconnection.

@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

@merlimat merlimat added this to the 2.8.0 milestone May 7, 2021
@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label May 7, 2021
@merlimat merlimat merged commit e213d35 into apache:master May 7, 2021
@BewareMyPower BewareMyPower deleted the bewaremypower/java-flow-zero branch May 8, 2021 02:32
eolivelli pushed a commit to eolivelli/pulsar that referenced this pull request May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

2 participants