[improve][broker]Improve the feature "Optimize subscription seek (cursor reset) by timestamp": search less entries #24219
+24
−10
Conversation
This file contains hidden or 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
…sor reset) by timestamp": search less entries
lhotari
reviewed
Apr 26, 2025
Member
lhotari
left a comment
lhotari
left a comment
There was a problem hiding this comment.
I haven't reviewed this yet, but we might need more real world test cases to get confidence that we don't cause regressions. Previous optimization changes caused regression #23910 which was addressed by #23919. The optimization was different that time, so perhaps things are fine with this optimization.
merlimat
approved these changes
Apr 26, 2025
Contributor
merlimat
left a comment
merlimat
left a comment
There was a problem hiding this comment.
The change looks good. I'm this case it's safe to skip the first ledger in the search since all the entries would have a smaller timestamp. Including last entry is enough.
lhotari
approved these changes
Apr 26, 2025
Technoboy-
approved these changes
Apr 27, 2025
manas-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 2, 2025
…sor reset) by timestamp": search less entries (apache#24219) (cherry picked from commit bb2e4ab) (cherry picked from commit 376bf6e)
srinath-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 6, 2025
…sor reset) by timestamp": search less entries (apache#24219) (cherry picked from commit bb2e4ab) (cherry picked from commit 376bf6e)
Contributor
Author
manas-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 7, 2025
…sor reset) by timestamp": search less entries (apache#24219) (cherry picked from commit bb2e4ab) (cherry picked from commit e7dece7)
manas-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 7, 2025
…sor reset) by timestamp": search less entries (apache#24219) (cherry picked from commit bb2e4ab) (cherry picked from commit e7dece7)
dao-jun
pushed a commit
to ascentstream/pulsar
that referenced
this pull request
May 8, 2025
…sor reset) by timestamp": search less entries (apache#24219) (cherry picked from commit bb2e4ab)
15 tasks
srinath-ctds
pushed a commit
to datastax/pulsar
that referenced
this pull request
May 12, 2025
…sor reset) by timestamp": search less entries (apache#24219) (cherry picked from commit bb2e4ab) (cherry picked from commit e7dece7)
KannarFr
pushed a commit
to CleverCloud/pulsar
that referenced
this pull request
Sep 22, 2025
…sor reset) by timestamp": search less entries (apache#24219)
walkinggo
pushed a commit
to walkinggo/pulsar
that referenced
this pull request
Oct 8, 2025
…sor reset) by timestamp": search less entries (apache#24219)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Motivation
Background
#22792 improved the behavior that searching messages by timestamp. For example:
There are ledgers as follows:
[{ "ledger": "1", "entries": "10", "closed_time": "1000" },{ "ledger": "2", "entries": "10", "closed_time": "1010" },{ "ledger": "3", "entries": "10", "closed_time": "3000" }]And the config
broker.conf -> managedLedgerCursorResetLedgerCloseTimestampMaxClockSkewMillisis20ms, when you search for timestamp1000, the broker will find ledgers whose timestamp is larger thanledger.closed_time-20and less thanledger.closed_time+20. Then the result range is[1:0 ~ 3:9].Modifications
Since we have the configuration
broker.conf -> managedLedgerCursorResetLedgerCloseTimestampMaxClockSkewMillis, the entries[1:0 ~ 1:8]can be skipped to search, because we have already given enough scope expanded bymanagedLedgerCursorResetLedgerCloseTimestampMaxClockSkewMillis, which improves the search.Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: x