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.

Sign up for GitHub

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

Jump to bottom

Trim deleted entries after recover cursor. #4987

Merged
merged 7 commits into from
Sep 6, 2019
Merged

Trim deleted entries after recover cursor. #4987

merged 7 commits into from
Sep 6, 2019

Conversation

codelipenghui
Copy link
Contributor

@codelipenghui codelipenghui commented Aug 20, 2019 •
edited
Loading

Motivation

The recovered cursor already has the marked individually deleted messages ranges and the last mark delete entry, for messages redelivery, messages that were marked for deletion before the recovery are not being replayed.

But for message delivery, broker will read entries after the last mark delete entry, have not handle
the marked individually deleted messages ranges. This PR will trim messages which are contains in the marked individually deleted messages ranges.

Modification

While the read position of the cursor is less than the upper of LastIndividualDeletedRange, already acked messages are not being dispatched, otherwise not.

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

  • Does this pull request introduce a new feature? (no)

@codelipenghui
Copy link
Contributor Author

retest this please

@sijie sijie added area/broker type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Aug 21, 2019
@sijie sijie added this to the 2.5.0 milestone Aug 21, 2019
@sijie
Copy link
Member

sijie commented Aug 23, 2019

@merlimat @rdhabalia PTAL

@merlimat
Copy link
Contributor

@codelipenghui

While broker recover a cursor, broker will dispatch all messages after the last mark delete entry, so if user has an early position not acked, broker will dispatch so many messages which is already acked by consumer.

The description is not accurate. The recovered cursor already has the marked individually deleted messages ranges. There is a max set on how many "ranges" to keep (for limiting the amount of memory).

Messages that were marked for deletion before the recovery are not being replayed.

@codelipenghui
Copy link
Contributor Author

@merlimat I have update the description, please take a look.

@sijie
Copy link
Member

sijie commented Aug 29, 2019

ping @merlimat

@sijie
Copy link
Member

sijie commented Sep 2, 2019

@codelipenghui can you rebase the pull request?

@codelipenghui
Copy link
Contributor Author

@sijie I have rebase the pull request, please take a look.

@codelipenghui
Copy link
Contributor Author

run cpp tests

@aahmed-se aahmed-se merged commit dc7d01e into apache:master Sep 6, 2019
@wolfstudy wolfstudy modified the milestones: 2.5.0, 2.4.2 Nov 19, 2019
@wolfstudy
Copy link
Member

Change the Milestone to 2.4.2, because of conflict.

wolfstudy pushed a commit that referenced this pull request Nov 20, 2019
@codelipenghui @wolfstudy
Trim deleted entries after recover cursor. (#4987)
297c606 
* Trim deleted entries after recover cursor.

* Fix errors

* Add managed cursor unit tests.

* Fix tests and handle cursor reset.

* fix unit tests

* Fix tests

* Fix check style

(cherry picked from commit dc7d01e)
@codelipenghui codelipenghui deleted the avoid_duplicate_dispatch branch May 19, 2021 05:31
eolivelli pushed a commit to datastax/pulsar that referenced this pull request Nov 24, 2021
@codelipenghui @eolivelli
Trim deleted entries after recover cursor. (apache#4987)
c4c0b6d 
* Trim deleted entries after recover cursor.

* Fix errors

* Add managed cursor unit tests.

* Fix tests and handle cursor reset.

* fix unit tests

* Fix tests

* Fix check style

(cherry picked from commit dc7d01e)
@lhotari
Copy link
Member

lhotari commented Dec 15, 2021

improved fix is #5894

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sijie sijie sijie approved these changes

@aahmed-se aahmed-se aahmed-se approved these changes

@merlimat merlimat Awaiting requested review from merlimat

@rdhabalia rdhabalia Awaiting requested review from rdhabalia

@jiazhai jiazhai Awaiting requested review from jiazhai

Assignees

@codelipenghui codelipenghui

Labels
area/broker type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Milestone
2.4.2
Development

Successfully merging this pull request may close these issues.
6 participants
@codelipenghui @sijie @merlimat @wolfstudy @lhotari @aahmed-se