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.

Sign up for GitHub

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

Jump to bottom

[improve][broker][PIP-149]Make resetCursor async #16355

Merged
merged 7 commits into from
Jul 13, 2022
Merged

[improve][broker][PIP-149]Make resetCursor async #16355

merged 7 commits into from
Jul 13, 2022

Conversation

AnonHxy
Copy link
Contributor

@AnonHxy AnonHxy commented Jul 3, 2022

Master Issue: #14365

Motivation

Modifications

  • Make resetCursor async

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc-not-needed

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 3, 2022
@AnonHxy
Copy link
Contributor Author

AnonHxy commented Jul 3, 2022

/pulsarbot run-failure-checks

1 similar comment
@AnonHxy
Copy link
Contributor Author

AnonHxy commented Jul 3, 2022

/pulsarbot run-failure-checks

@codelipenghui codelipenghui added this to the 2.11.0 milestone Jul 4, 2022
@codelipenghui codelipenghui added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker labels Jul 4, 2022
@AnonHxy
Copy link
Contributor Author

AnonHxy commented Jul 4, 2022

/pulsarbot run-failure-checks

@AnonHxy
Copy link
Contributor Author

AnonHxy commented Jul 6, 2022

/pulsarbot run-failure-checks

@HQebupt
Copy link
Contributor

HQebupt commented Jul 6, 2022

LGTM

Jason918
Jason918 approved these changes Jul 8, 2022
View reviewed changes
Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

@Technoboy- Technoboy- merged commit ea417fb into apache:master Jul 13, 2022
wuxuanqicn pushed a commit to wuxuanqicn/pulsar that referenced this pull request Jul 14, 2022
@AnonHxy @weimob-wuxuanqi
[improve][broker][PIP-149]Make resetCursor async (apache#16355)
f7eb2c5
@nodece nodece mentioned this pull request Jul 21, 2022
Merged
3 tasks
@codelipenghui
Copy link
Contributor

@nodece We haven't cherry-picked any PRs of PIP-149 since there are lots of conflicts that might introduce new problems to the release branch. If the issue can be reproduced by pulsarctl, we'd better push a hot fix to branch-2.10 directly.

@nodece
Copy link
Member

nodece commented Jul 25, 2022

@codelipenghui Thanks for your explanation! There has a waiting thread, you can see this log:

    reset_cursor_test.go:64: code: 500 reason: 
         --- An unexpected error occurred in the server ---
        Message: null
        Stacktrace:
        java.util.concurrent.TimeoutException
        	at java.base/java.util.concurrent.CompletableFuture.timedGet(CompletableFuture.java:1886)
        	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2021)
        	at org.apache.pulsar.broker.admin.impl.PersistentTopicsBase.getTopicReference(PersistentTopicsBase.java:4085)
        	at org.apache.pulsar.broker.admin.impl.PersistentTopicsBase.internalResetCursorForNonPartitionedTopic(PersistentTopicsBase.java:2135)
        	at org.apache.pulsar.broker.admin.impl.PersistentTopicsBase.lambda$internalResetCursor$156(PersistentTopicsBase.java:2112)
        	at java.base/java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:714)
        	at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:506)
        	at java.base/java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2073)
        	at org.apache.pulsar.metadata.impl.ZKMetadataStore.handleGetResult(ZKMetadataStore.java:233)
        	at org.apache.pulsar.metadata.impl.ZKMetadataStore.lambda$batchOperation$6(ZKMetadataStore.java:183)
        	at org.apache.pulsar.metadata.impl.PulsarZooKeeperClient$3$1.processResult(PulsarZooKeeperClient.java:490)
        	at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:722)
        	at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:563)

I'll make a PR to branch-2.10 for fix this issue.

nodece pushed a commit to nodece/pulsar that referenced this pull request Jul 25, 2022
@AnonHxy @nodece
[improve][broker][PIP-149]Make resetCursor async (apache#16355)
1929e6a 
(cherry picked from commit ea417fb)
Signed-off-by: Zixuan Liu <[email protected]>
@nodece nodece mentioned this pull request Jul 25, 2022
Merged
4 tasks
nodece added a commit that referenced this pull request Jul 26, 2022
@nodece @AnonHxy
[improve][broker][PIP-149]Make resetCursor async (#16355) (#16774)
e15a51f 
(cherry picked from commit ea417fb)
Signed-off-by: Zixuan Liu <[email protected]>

Co-authored-by: Xiaoyu Hou <[email protected]>
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2022
@nodece @nicoloboschi
[improve][broker][PIP-149]Make resetCursor async (apache#16355) (apac…
d36c400 
…he#16774)

(cherry picked from commit ea417fb)
Signed-off-by: Zixuan Liu <[email protected]>

Co-authored-by: Xiaoyu Hou <[email protected]>
(cherry picked from commit e15a51f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codelipenghui codelipenghui codelipenghui left review comments

@Technoboy- Technoboy- Technoboy- approved these changes

@HQebupt HQebupt HQebupt approved these changes

@Jason918 Jason918 Jason918 approved these changes

@nodece nodece Awaiting requested review from nodece

@mattisonchao mattisonchao Awaiting requested review from mattisonchao

Assignees

@AnonHxy AnonHxy

Labels
area/broker cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.10.2 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Milestone
2.11.0
Development

Successfully merging this pull request may close these issues.
6 participants
@AnonHxy @HQebupt @codelipenghui @nodece @Jason918 @Technoboy-