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

Clearing message queues after seek requests #419

Merged
merged 2 commits into from
Jan 26, 2021
Merged

Clearing message queues after seek requests #419

merged 2 commits into from
Jan 26, 2021

Conversation

milos-matijasevic
Copy link
Contributor

@milos-matijasevic milos-matijasevic commented Dec 20, 2020

Motivation

Message queues should be cleared after seek requests (Seek and SeekByTime). If this is not performed messages that stay in message queues will be consumed before the sough message.

Modifications

Cleaning queueCh and messageCh after successful seek request in partition_consumer.go.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as TestConsumerSeekByTime and TestConsumerSeek.

This change added tests and can be verified as follows:

  • Extended TestConsumerSeekByTime and TestConsumerSeek test for consuming correctly sought message even if some messages stay in message queues.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)

Documentation

  • Does this pull request introduce a new feature? (no)

Signed-off-by: milos-matijasevic <milosmatijasevic2015@gmail.com>
Signed-off-by: milos-matijasevic <milosmatijasevic2015@gmail.com>
@wolfstudy wolfstudy requested review from jiazhai, wolfstudy and merlimat and removed request for jiazhai and wolfstudy January 4, 2021 11:12
@wolfstudy wolfstudy added this to the 0.4.0 milestone Jan 4, 2021
@wolfstudy wolfstudy self-requested a review January 4, 2021 11:15
@jiazhai jiazhai merged commit c24191b into apache:master Jan 26, 2021
rueian added a commit to rueian/pulsar-client-go that referenced this pull request Feb 15, 2021
Fixes apache#356
Fixes apache#419

Motivtions

The changes made by the original PR apache#329 are no longer works.
This PR is trying to fix the bugs and make the test case be more robust.

Moditications

* fix the wrong `pc.startMessageID == lastestMessageID` with `pc.startMessageID.equal(lastestMessageID.(messageID))`
* fix the hanging `pc.requestSeek(msgID.messageID)` with `pc.requestSeekWithoutClear(msgID.messageID)` before the dispatch loop
* make `HasNext()` return true in case of `StartMessageIdInclusive` and `LastestMessageID`
* make `TestReaderLatestInclusiveHasNext` test case be more robust
rueian added a commit to rueian/pulsar-client-go that referenced this pull request Feb 16, 2021
Fixes apache#356
Fixes apache#419

Motivtions

The changes made by the original PR apache#329 are no longer works.
This PR is trying to fix the bugs and make the test case be more robust.

Moditications

* fix the wrong `pc.startMessageID == lastestMessageID` with `pc.startMessageID.equal(lastestMessageID.(messageID))`
* fix the hanging `pc.requestSeek(msgID.messageID)` with `pc.requestSeekWithoutClear(msgID.messageID)` before the dispatch loop
* make `TestReaderLatestInclusiveHasNext` test case be more robust
rueian added a commit to rueian/pulsar-client-go that referenced this pull request Feb 16, 2021
Fixes apache#356
Fixes apache#419

Motivtions

The changes made by the original PR apache#329 are no longer works.
This PR is trying to fix the bugs and make the test case be more robust.

Moditications

* fix the wrong `pc.startMessageID == lastestMessageID` with `pc.startMessageID.equal(lastestMessageID.(messageID))`
* fix the hanging `pc.requestSeek(msgID.messageID)` with `pc.requestSeekWithoutClear(msgID.messageID)` before the dispatch loop
* make the `HasNext()` return true in case of `StartMessageIdInclusive` and `LastestMessageID`
* make the `TestReaderLatestInclusiveHasNext` test case be more robust
rueian added a commit to rueian/pulsar-client-go that referenced this pull request Feb 16, 2021
Fixes apache#356
Fixes apache#419

Motivtions

The changes made by the original PR apache#329 are no longer works.
This PR is trying to fix the bugs and make the test case be more robust.

Moditications

* fix the wrong `pc.startMessageID == lastestMessageID` with `pc.startMessageID.equal(lastestMessageID.(messageID))`
* fix the hanging `pc.requestSeek(msgID.messageID)` with `pc.requestSeekWithoutClear(msgID.messageID)` before the dispatch loop
* make the `TestReaderLatestInclusiveHasNext` test case be more robust
rueian added a commit to rueian/pulsar-client-go that referenced this pull request Feb 16, 2021
Fixes apache#356
Fixes apache#419

Motivtions

The changes made by the original PR apache#329 are no longer works.
This PR is trying to fix the bugs and make the test case be more robust.

Moditications

* fix the wrong comparison `pc.startMessageID == lastestMessageID` with `pc.startMessageID.equal(lastestMessageID.(messageID))`
* fix the hanging `pc.requestSeek(msgID.messageID)` with `pc.requestSeekWithoutClear(msgID.messageID)` before the dispatch loop
* make the `TestReaderLatestInclusiveHasNext` test case be more robust
wolfstudy pushed a commit that referenced this pull request Feb 19, 2021
* Fix reader with start latest message id inclusive (#329)

Fixes #356
Fixes #419

Motivtions

The changes made by the original PR #329 are no longer works.
This PR is trying to fix the bugs and make the test case be more robust.

Moditications

* fix the wrong comparison `pc.startMessageID == lastestMessageID` with `pc.startMessageID.equal(lastestMessageID.(messageID))`
* fix the hanging `pc.requestSeek(msgID.messageID)` with `pc.requestSeekWithoutClear(msgID.messageID)` before the dispatch loop
* make the `TestReaderLatestInclusiveHasNext` test case be more robust

* Renew the expired certs in integration-tests with script
@milos-matijasevic milos-matijasevic deleted the clear-ch-after-seek branch February 20, 2021 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants