Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[fix][client] Fix Reader.hasMessageAvailable return wrong value after seeking by timestamp with startMessageIdInclusive #23502

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

summeriiii
Copy link
Contributor

Fixes #23501

Motivation

When seek to the current time reader.seek(System.currentTimeMillis());, reader.hasMessageAvailable() should return false cause there has no message to read, but right now it returns true.

Modifications

  • when hasSoughtByTimestamp, we only need to check result < 0
else if (hasSoughtByTimestamp) {
      completehasMessageAvailableWithValue(booleanFuture, result < 0);
}

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

… seeking by timestamp with startMessageIdInclusive
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 22, 2024
@summeriiii
Copy link
Contributor Author

@BewareMyPower @lhotari PTAL, thanks

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution @summeriiii

@summeriiii
Copy link
Contributor Author

/pulsarbot run-failure-checks

@BewareMyPower BewareMyPower added the type/bug The PR fixed a bug or issue reported a bug label Oct 24, 2024
@BewareMyPower
Copy link
Contributor

The failed flaky test is #23474, let's merge this PR first

@BewareMyPower BewareMyPower merged commit fcb3592 into apache:master Oct 24, 2024
52 of 53 checks passed
lhotari pushed a commit that referenced this pull request Oct 30, 2024
… seeking by timestamp with startMessageIdInclusive (#23502)

(cherry picked from commit fcb3592)
lhotari pushed a commit that referenced this pull request Oct 30, 2024
… seeking by timestamp with startMessageIdInclusive (#23502)

(cherry picked from commit fcb3592)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 5, 2024
… seeking by timestamp with startMessageIdInclusive (apache#23502)

(cherry picked from commit fcb3592)
(cherry picked from commit b7b69dc)
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Nov 6, 2024
… seeking by timestamp with startMessageIdInclusive (apache#23502)

(cherry picked from commit fcb3592)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 7, 2024
… seeking by timestamp with startMessageIdInclusive (apache#23502)

(cherry picked from commit fcb3592)
(cherry picked from commit b7b69dc)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 7, 2024
… seeking by timestamp with startMessageIdInclusive (apache#23502)

(cherry picked from commit fcb3592)
(cherry picked from commit b7b69dc)
lhotari pushed a commit that referenced this pull request Nov 13, 2024
… seeking by timestamp with startMessageIdInclusive (#23502)

(cherry picked from commit fcb3592)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Reader.hasMessageAvailable return wrong value after seeking by timestamp with startMessageIdInclusive
3 participants