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

[improve][broker]prevent partitioned metadata lookup request when broker is closing #17315

Merged
merged 5 commits into from
Sep 15, 2022

Conversation

HQebupt
Copy link
Contributor

@HQebupt HQebupt commented Aug 28, 2022

Motivation

Inspired by #17085
When the leader broker is closing, and the client will reconnect and redo lookup, resulting in unexpected results.

@codelipenghui
Looks like we also need to check other cases (e.g. handleConnect)
If the broker is shutting down, we should also prevent new connections.

Therefore, it is necessary to check the status of the broker when handling requests, including connect\lookup\partitioned metadata.

The following is the interaction workflow between consumer and broker.

Consumer -> Broker: CONNECT
Broker -> Consumer: CONNECTED
Consumer -> Broker: PARTITIONED_METADATA
Broker -> Consumer: PARTITIONED_METADATA_RESPONSE
Consumer -> Broker: LOOKUP
Broker -> Consumer: LOOKUP_RESPONSE
Consumer -> Broker: SUBSCIBE
Broker -> Consumer: SUCCESS
Consumer -> Broker: FLOW
Broker -> Consumer: MESSAGE
Consumer - Broker: ACK

The following screenshot is the interaction workflow between producer and broker.
image

Modifications

  • Check Pulsar Service state when handle connection and partitioned metadata request.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc-not-needed

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 28, 2022
@AnonHxy AnonHxy self-requested a review August 28, 2022 07:58
@HQebupt
Copy link
Contributor Author

HQebupt commented Aug 29, 2022

/pulsarbot run-failure-checks

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Please add a test to cover the new changes.

@HQebupt HQebupt force-pushed the handlePartitionMetadaEx branch from 079efbb to 2258e57 Compare September 10, 2022 15:11
@HQebupt
Copy link
Contributor Author

HQebupt commented Sep 10, 2022

@codelipenghui PTAL when you are available.

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

LGTM

@AnonHxy
Copy link
Contributor

AnonHxy commented Sep 13, 2022

LGTM with minor comment

@AnonHxy AnonHxy merged commit 256bd68 into apache:master Sep 15, 2022
@Technoboy- Technoboy- removed this from the 2.12.0 milestone Sep 26, 2022
@Technoboy- Technoboy- added this to the 2.11.0 milestone Sep 26, 2022
Technoboy- pushed a commit that referenced this pull request Sep 26, 2022
dragonls pushed a commit to dragonls/pulsar that referenced this pull request Oct 21, 2022
dragonls pushed a commit to dragonls/pulsar that referenced this pull request Oct 21, 2022
…2.9-wxg-release' (merge request !53)

[improve][broker] fix broker irrational behavior when it is closing (apache#17085)
1. [[improve][broker]fix broker irrational behavior when it is closing by HQebupt · Pull Request apache#17085 · apache/pulsar](apache#17085)
2. [[improve][broker]prevent new connection request when broker is closing by HQebupt · Pull Request apache#17314 · apache/pulsar](apache#17314)
3. [[improve][broker]prevent partitioned metadata lookup request when broker is closing by HQebupt · Pull Request apache#17315 · apache/pulsar](apache#17315)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants