Data is not deleted after expiration due to connected readers #5621
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
*Problem* A problem is observed when stress testing pulsar using [pulsar-flink](https://github.com/streamnative/pulsar-flink) - No matter what TTL or retention setting is used, the data is never cleaned up. So the stress test ends up failing due to disk filled up. The root cause of the problem is described as below. when a reader is opened using `MessageId.earliest`, a non-durable cursor with position (-1, -2) is added to the cursor heap. The position `(-1, -2)` in the heap is never updated because non-durable cursors are never advanced when mark-deletions happen. So the slowest cursor position is always `(-1, -2)`, thus causing no ledger can be deleted even they are expired or over quota. *Motivation* Fix the problem to make sure Pulsar honor to TTL and retention settings. *Modifications* - Fix the `startPosition` when PersistentTopic opens a non-durable cursor on `MessageId.earliest`. So the `startPosition` is (-1, -1) not (-1, -2). - Fix the `NonDurableCursorImpl` constructor to check if the position in the ledger of `MessageId.earliest`. If the provided position is in the `earliest` ledger, the mark-deleted position will be set to the previous position of first position. - Fix the `NonDurableCursorImpl` to advance ledger cursor when mark-deletion happens on a non-durable cursor. *Verify this change* Unit tests are coming.
sijie
requested review from
merlimat,
rdhabalia,
jiazhai,
codelipenghui and
wolfstudy
|
this pull request is now ready for review. The unit test is added to reproduce the issue and used for verifying the fix works. |
|
This pull request also fixes the root cause of #5558 |
merlimat
approved these changes
Nov 11, 2019
|
run java8 tests |
codelipenghui
approved these changes
Nov 12, 2019
|
Seems failed unit tests are related to this change. |
|
@codelipenghui fixed the NonDurableCursorTest |
|
run java8 tests |
|
run cpp tests |
|
retest this please |
|
run integration tests |
|
run java8 tests |
|
run java8 tests |
wolfstudy
pushed a commit
that referenced
this pull request
Nov 20, 2019
* Data is not deleted after expiration due to connected readers *Problem* A problem is observed when stress testing pulsar using [pulsar-flink](https://github.com/streamnative/pulsar-flink) - No matter what TTL or retention setting is used, the data is never cleaned up. So the stress test ends up failing due to disk filled up. The root cause of the problem is described as below. when a reader is opened using `MessageId.earliest`, a non-durable cursor with position (-1, -2) is added to the cursor heap. The position `(-1, -2)` in the heap is never updated because non-durable cursors are never advanced when mark-deletions happen. So the slowest cursor position is always `(-1, -2)`, thus causing no ledger can be deleted even they are expired or over quota. *Motivation* Fix the problem to make sure Pulsar honor to TTL and retention settings. *Modifications* - Fix the `startPosition` when PersistentTopic opens a non-durable cursor on `MessageId.earliest`. So the `startPosition` is (-1, -1) not (-1, -2). - Fix the `NonDurableCursorImpl` constructor to check if the position in the ledger of `MessageId.earliest`. If the provided position is in the `earliest` ledger, the mark-deleted position will be set to the previous position of first position. - Fix the `NonDurableCursorImpl` to advance ledger cursor when mark-deletion happens on a non-durable cursor. *Verify this change* Unit tests are coming. (cherry picked from commit 3e7cb68)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
A problem is observed when stress testing pulsar using pulsar-flink -
No matter what TTL or retention setting is used, the data is never cleaned up. So the stress test ends up failing due
to disk filled up.
The root cause of the problem is described as below.
when a reader is opened using
MessageId.earliest, a non-durable cursor with position (-1, -2) is added to the cursor heap.The position
(-1, -2)in the heap is never updated because non-durable cursors are never advanced when mark-deletionshappen. So the slowest cursor position is always
(-1, -2), thus causing no ledger can be deleted even they are expiredor over quota.
Motivation
Fixes #5558
Fix the problem to make sure Pulsar honor to TTL and retention settings.
Modifications
Fix the
startPositionwhen PersistentTopic opens a non-durable cursor onMessageId.earliest.So the
startPositionis (-1, -1) not (-1, -2).Fix the
NonDurableCursorImplconstructor to check if the position in the ledger ofMessageId.earliest.If the provided position is in the
earliestledger, the mark-deleted position will be set to the previousposition of first position.
Fix the
NonDurableCursorImplto advance ledger cursor when mark-deletion happens on a non-durable cursor.Verify this change
Add a unit test to simulate the mixture of durable and non-durable cursors, and verify the fix address the problem.