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

[Issue 8260] Support reset cursor to a batch index of the batching message #8285

Merged
merged 30 commits into from
Oct 21, 2020

Conversation

Renkai
Copy link
Contributor

@Renkai Renkai commented Oct 17, 2020

Signed-off-by: Renkai gaelookair@gmail.com

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #8260

Motivation

Make reset cursor command able to reset to a specific index in batch mode.

Modifications

Now reset offset command change index info on broker side

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

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

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (yes / no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
@Renkai Renkai marked this pull request as ready for review October 18, 2020 09:57
@Renkai
Copy link
Contributor Author

Renkai commented Oct 18, 2020

Still have some problem, unit test written by me can't be passed(due to timeout),
https://github.com/apache/pulsar/pull/8285/files#diff-30ab326ad2d2e0d984b321af3c68c7bb7ab345fac245bbcc6739f58daffc0e68R120

Maybe there is some thing wrong with my test configuration

mvn -pl pulsar-broker -Dtest=SubscriptionSeekTest\#testSeekForBatch test -DredirectTestOutputToFile=false

PTAL @codelipenghui

@jiazhai
Copy link
Member

jiazhai commented Oct 19, 2020

/pulsarbot run-failure-checks

Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
@Renkai
Copy link
Contributor Author

Renkai commented Oct 19, 2020

/pulsarbot run-failure-checks

1 similar comment
@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

I have double-checked the implementation of the Batch Index Acknowledgment. Looks we can only pass the ackset to the broker from the client-side, because currently true in the BitSet represent the message does not acked, and false means the batch index are acked. So I think it's the root cause of why the consumer start receive message from the next message of the reset position. You can refer to

BitSetRecyclable bitSet = BitSetRecyclable.create();
bitSet.set(0, batchSize);
bitSet.clear(0, batchIndex + 1);
to check the current implementation.

Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
@Renkai
Copy link
Contributor Author

Renkai commented Oct 20, 2020

/pulsarbot run-failure-checks

Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
@Renkai
Copy link
Contributor Author

Renkai commented Oct 20, 2020

/pulsarbot run-failure-checks

Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
@wolfstudy
Copy link
Member

ping @codelipenghui PTAL again thanks.

Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
Signed-off-by: Renkai <gaelookair@gmail.com>
@Renkai
Copy link
Contributor Author

Renkai commented Oct 21, 2020

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit a5b4146 into apache:master Oct 21, 2020
codelipenghui pushed a commit that referenced this pull request Oct 22, 2020
…ssage (#8285)

### Motivation

Make reset cursor command able to reset to a specific index in batch mode.

### Modifications

Now reset offset command change index info on broker side

(cherry picked from commit a5b4146)
@codelipenghui
Copy link
Contributor

cherry-picked to branch-2.6(2.6.2)

@Renkai Renkai deleted the reset_batch_index branch October 22, 2020 07:14
wolfstudy added a commit that referenced this pull request Oct 30, 2020
wolfstudy pushed a commit that referenced this pull request Oct 30, 2020
…ssage (#8285)

### Motivation

Make reset cursor command able to reset to a specific index in batch mode.

### Modifications

Now reset offset command change index info on broker side

(cherry picked from commit a5b4146)
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Nov 13, 2020
…ssage (apache#8285)

### Motivation

Make reset cursor command able to reset to a specific index in batch mode.

### Modifications

Now reset offset command change index info on broker side
flowchartsman pushed a commit to flowchartsman/pulsar that referenced this pull request Nov 17, 2020
…ssage (apache#8285)

### Motivation

Make reset cursor command able to reset to a specific index in batch mode.

### Modifications

Now reset offset command change index info on broker side
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.

Support reset cursor to a batch index of the batching message
4 participants