Skip to content
This repository has been archived by the owner on Apr 1, 2024. It is now read-only.

ISSUE-19808: [Bug] org.apache.bookkeeper.mledger.impl.OpFindNewest#readEntryComplete #5622

Open
2 tasks done
sijie opened this issue Mar 14, 2023 · 0 comments
Open
2 tasks done
Labels

Comments

@sijie
Copy link
Member

sijie commented Mar 14, 2023

Original Issue: apache#19808


Search before asking

  • I searched in the issues and found nothing similar.

Version

2.11
org.apache.bookkeeper.mledger.impl.OpFindNewest#readEntryComplete

Minimal reproduce step

image

1.state = checkFirst, condition.apply(entry) = true
The following code is executed:

lastMatchedPosition = position;
                // check last entry
                state = State.checkLast;
                PositionImpl lastPosition = ledger.getLastPosition();
                searchPosition = ledger.getPositionAfterN(searchPosition, max, PositionBound.startExcluded);
                if (lastPosition.compareTo(searchPosition) < 0) {
                    if (log.isDebugEnabled()) {
                        log.debug("first position {} matches, last should be {}, but moving to lastPos {}", position,
                                searchPosition, lastPosition);
                    }
                    searchPosition = lastPosition;
                }
                find();

crux of the problem:
The calculated searchPosition may be equal to the current entry position, then, after calling find(), state=checkLast is executed the next time, this causes condition.apply(entry) to be called twice, and with the same entry.

What did you expect to see?

I'd like to add a check code, for example:

if (position.equals(searchPosition)) {
                    callback.findEntryComplete(position, OpFindNewest.this.ctx);
                    return;
                }
        case checkFirst:
            if (!condition.apply(entry)) {
                // If no entry is found that matches the condition, it is expected to pass null to the callback.
                // Otherwise, a message before the expiration date will be deleted due to message TTL.
                // cf. https://github.com/apache/pulsar/issues/5579
                callback.findEntryComplete(null, OpFindNewest.this.ctx);
                return;
            } else {
                lastMatchedPosition = position;
                // check last entry
                state = State.checkLast;
                PositionImpl lastPosition = ledger.getLastPosition();
                searchPosition = ledger.getPositionAfterN(searchPosition, max, PositionBound.startExcluded);
                if (position.equals(searchPosition)) {
                    callback.findEntryComplete(position, OpFindNewest.this.ctx);
                    return;
                }
                if (lastPosition.compareTo(searchPosition) < 0) {
                    if (log.isDebugEnabled()) {
                        log.debug("first position {} matches, last should be {}, but moving to lastPos {}", position,
                                searchPosition, lastPosition);
                    }
                    searchPosition = lastPosition;
                }
                find();
            }
            break;

What did you see instead?

condition.apply(entry) is executed twice

Anything else?

no

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@sijie sijie added the type/bug label Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant