Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[IMPROVED] Optimized calculating source sequences for sourced stream with lots of deletes. #6461

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

derekcollison
Copy link
Member

Added LoadPrevMsg to optimize when we walk backwards looking for source sequences and we have lots of interior deletes.

Signed-off-by: Derek Collison derek@nats.io

…ce sequences and we have lots of interior deletes.

Signed-off-by: Derek Collison <derek@nats.io>
@derekcollison derekcollison requested a review from a team as a code owner February 5, 2025 22:48
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, just one minor thing stood out.

if err != nil || len(sm.hdr) == 0 {
for seq := state.LastSeq; seq >= state.FirstSeq; {
sm, err := mset.store.LoadPrevMsg(seq, &smv)
if err == ErrStoreEOF || err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a tautological condition, if the err is ErrStoreEOF then it's also not nil. Is the EOF meant to be handled differently?

server/stream.go Show resolved Hide resolved
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@derekcollison derekcollison merged commit b484711 into main Feb 5, 2025
5 checks passed
@derekcollison derekcollison deleted the load-prev branch February 5, 2025 23:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants