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] Avoid subscription fenced error with consumer.seek whenever possible #23163

Merged
merged 5 commits into from
Aug 14, 2024

Conversation

poorbarcode
Copy link
Contributor

@poorbarcode poorbarcode commented Aug 13, 2024

Motivation

After a consumer calls seek, the operation reconnect will get a fenced error, which leads to more unnecessary reconnects. The steps to produce are as follows:

  • client-side: consumer connected
  • client-side: calls consumer.seek or seekAsync
  • broker-side:
    • mark subscription as fenced
    • remove all consumer
    • do reset the cursor internally
  • client-side:
    • consumer tries to reconnect
    • get an error Subscription was fenced due to the task reset cursor not finished yet.
    • loop...
  • broker-side: reset cursor is completed
  • client-side: The consumer tries to reconnect, and it will finish at this time.

Modifications

  • This PR fixes the issue that a consumer fenced itself.
  • This PR can probably fix the issue that a consumer.seek fenced other consumers under the same subscription.
    • This PR does not fix all scenarios in which a consumer.seek fenced other consumers under the same subscription. See the following example:
time broker-side: consumer_1 broker-side: consumer_2
1 check whether an in-progress reset cursor exists: false
2 start to add consumer_2
3 reset cursor started
4 mark subscription as fenced
5 get a fenced error

I do not want to improve the behavior above because it needs a lock. Since it does not cause critical errors, it just causes reconnection and the possibility of it occurring is low, we can leave it there.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost release/3.2.5 release/3.0.7 release/3.3.2 labels Aug 13, 2024
@poorbarcode poorbarcode added this to the 3.4.0 milestone Aug 13, 2024
@poorbarcode poorbarcode self-assigned this Aug 13, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 13, 2024
@lhotari lhotari changed the title [improve] [broker] Avoid subsscription fenced error that introduced by consumer.seek whenever possible [improve] [broker] Avoid subscription fenced error with consumer.seek whenever possible Aug 13, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Good work! I added some review comments.

@lhotari lhotari merged commit d5ce1ce into apache:master Aug 14, 2024
57 checks passed
lhotari pushed a commit that referenced this pull request Aug 14, 2024
lhotari pushed a commit that referenced this pull request Aug 14, 2024
lhotari pushed a commit that referenced this pull request Aug 16, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2024
… whenever possible (apache#23163)

(cherry picked from commit d5ce1ce)
(cherry picked from commit bbe67c8)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2024
… whenever possible (apache#23163)

(cherry picked from commit d5ce1ce)
(cherry picked from commit bbe67c8)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 20, 2024
… whenever possible (apache#23163)

(cherry picked from commit d5ce1ce)
(cherry picked from commit bbe67c8)
grssam pushed a commit to grssam/pulsar that referenced this pull request Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/reliability The function does not work properly in certain specific environments or failures. e.g. data lost cherry-picked/branch-3.0 cherry-picked/branch-3.2 cherry-picked/branch-3.3 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.7 release/3.2.5 release/3.3.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants