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 issues in advanceNonDurableCursors #10667

Merged
merged 7 commits into from
May 25, 2021

Conversation

jerrypeng
Copy link
Contributor

Motivation

There are a couple of issues in advanceNonDurableCursors

  1. Durable cursors unnecessarily run through this logic
  2. For Non-durable and durable cursors that are caught up reading the topic. The following warning message can be printed
message: [public/default/persistent/test] Failed to mark delete while trimming data ledgers: Invalid mark deleted position

This happens because highestPositionToDelete calculated when the reader/consumer is caught up is going to be higher than the last confirmed for the of the managed ledger. There is also no point in trying to advance the mark delete position if the reader/consumer is caught up.

@jerrypeng jerrypeng added type/bug The PR fixed a bug or issue reported a bug area/broker labels May 21, 2021
@jerrypeng jerrypeng self-assigned this May 21, 2021
@jerrypeng jerrypeng added this to the 2.8.0 milestone May 21, 2021
@eolivelli
Copy link
Contributor

Do we have tests that exercise explicitly this code?

@jerrypeng
Copy link
Contributor Author

@eolivelli yes in general but don't have a test to replicate this specific issue. Attempted to replicate issue in test but no a clear way how to

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@jerrypeng jerrypeng merged commit a244ed3 into apache:master May 25, 2021
Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

LGTM

yangl pushed a commit to yangl/pulsar that referenced this pull request Jun 23, 2021
Co-authored-by: Jerry Peng <jerryp@splunk.com>
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Nov 10, 2021
Co-authored-by: Jerry Peng <jerryp@splunk.com>
(cherry picked from commit a244ed3)
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Co-authored-by: Jerry Peng <jerryp@splunk.com>
Technoboy- pushed a commit that referenced this pull request Jul 5, 2022
Co-authored-by: Jerry Peng <jerryp@splunk.com>
@Technoboy- Technoboy- added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Jul 5, 2022
@Technoboy-
Copy link
Contributor

Cherry-pick this to branch-2.7, because we need to cherry-pick #12602, it relies on this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker cherry-picked/branch-2.7 Archived: 2.7 is end of life 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.

5 participants