coderzc commented on code in PR #21187:
URL: https://github.com/apache/pulsar/pull/21187#discussion_r1464262263
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -2519,7 +2525,8 @@ public Position getPersistentMarkDeletedPosition() {
public void rewind() {
lock.writeLock().lock();
try {
- PositionImpl newReadPosition =
ledger.getNextValidPosition(markDeletePosition);
+ PositionImpl newReadPosition = isCursorReadFromCompacted() ?
markDeletePosition.getNext() :
+ ledger.getNextValidPosition(markDeletePosition);
Review Comment:
@codelipenghui
> After checking the changes again. It looks like we don't need to persist
the readCompacted? If depends on the consumer settings. We only need to set the
readCompacted when new consumer connected?
I don't think this will work well because when recovering the cursor, maybe
the consumer is not connected yet, then our md-position will be forced to move
to the first position of the ml.
please see:
https://github.com/apache/pulsar/pull/21187/files#diff-fad355f91bd15cc041161f9a46fce62b7fee87fbfb8f0ff8a8b724a1bd1f29eeR685-R695
> Oh, I found a similar PR which fixed the Reader API.
Yes, they are a issue
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -2519,7 +2525,8 @@ public Position getPersistentMarkDeletedPosition() {
public void rewind() {
lock.writeLock().lock();
try {
- PositionImpl newReadPosition =
ledger.getNextValidPosition(markDeletePosition);
+ PositionImpl newReadPosition = isCursorReadFromCompacted() ?
markDeletePosition.getNext() :
+ ledger.getNextValidPosition(markDeletePosition);
Review Comment:
@codelipenghui
> After checking the changes again. It looks like we don't need to persist
the readCompacted? If depends on the consumer settings. We only need to set the
readCompacted when new consumer connected?
I don't think this will work well because when recovering the cursor, maybe
the consumer is not connected yet, then our md-position will be forced to move
to the first position of the ml.
please see:
https://github.com/apache/pulsar/pull/21187/files#diff-fad355f91bd15cc041161f9a46fce62b7fee87fbfb8f0ff8a8b724a1bd1f29eeR685-R695
> Oh, I found a similar PR which fixed the Reader API.
Yes, they are a issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
