[Broker] Fix call sync method in async rest api for `internalDeletePartitionedTopic` #13805

mattisonchao · 2022-01-18T08:31:47Z

Motivation

Avoid call sync method in async rest API for PersistentTopicsBase#internalDeletePartitionedTopic.

Modifications

Use async instead of sync method.

Verifying this change

Make sure that the change passes the CI checks.

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: ( 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

no-need-doc

…artitionedTopic``.

fixes style issue.

mattisonchao · 2022-01-19T01:59:00Z

/pulsarbot rerun-failure-checks

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java

This reverts commit fbb0aaf.

mattisonchao · 2022-01-19T09:27:06Z

/pulsarbot rerun-failure-checks

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java

…of completableFuture exception.

mattisonchao · 2022-01-20T07:19:52Z

/pulsarbot rerun-failure-checks

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java

pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java

pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java

…l/PersistentTopicsBase.java Co-authored-by: lipenghui <[email protected]>

…reUtil.java Co-authored-by: lipenghui <[email protected]>

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java

mattisonchao · 2022-01-25T23:47:05Z

/pulsarbot rerun-failure-checks

mattisonchao · 2022-01-26T01:17:31Z

@codelipenghui @Technoboy- PTAL :-)

Technoboy-

LGTM

codelipenghui · 2022-01-26T05:20:44Z

pulsar-common/src/main/java/org/apache/pulsar/common/util/FutureUtil.java

+        if (ex instanceof CompletionException) {
+            return ex.getCause();
+        } else if (ex instanceof ExecutionException) {
+            return ex.getCause();


Why cancel recursion?

Because the source code of CompletableFuture

https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/java.base/share/classes/java/util/concurrent/CompletableFuture.java#L323-L348

The CompletionException won't be wrapped twice.

mattisonchao and others added 2 commits January 18, 2022 16:26

[Broker] Fix call sync method in async rest api for ``internalDeleteP…

4168b9c

…artitionedTopic``.

Update PulsarWebResource.java

269b6eb

fixes style issue.

mattisonchao changed the title ~~[Broker] Fix call sync method in async rest api for ``internalDeleteP…~~ [Broker] Fix call sync method in async rest api for ``internalDeletePartitionedTopic Jan 18, 2022

mattisonchao changed the title ~~[Broker] Fix call sync method in async rest api for ``internalDeletePartitionedTopic~~ [Broker] Fix call sync method in async rest api for internalDeletePartitionedTopic Jan 18, 2022

github-actions bot assigned mattisonchao Jan 18, 2022

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 18, 2022

HQebupt approved these changes Jan 18, 2022

View reviewed changes

codelipenghui added this to the 2.10.0 milestone Jan 18, 2022

codelipenghui added release/2.9.3 release/2.8.3 labels Jan 18, 2022

fixes checkstyle

19f533e

Jason918 reviewed Jan 19, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated Show resolved Hide resolved

mattisonchao added 2 commits January 19, 2022 15:30

Use thenAccept instead of new CompletableFuture

fbb0aaf

Revert "Use thenAccept instead of new CompletableFuture"

702545e

This reverts commit fbb0aaf.

Change apply to accept

be05da0

Jason918 reviewed Jan 19, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated Show resolved Hide resolved

fixes wrong error catch.

c6ab9fd

Jason918 requested changes Jan 20, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/web/PulsarWebResource.java Outdated Show resolved Hide resolved

Add FutureUtil.getOriginalException to hanlde various situations …

74ccf9f

…of completableFuture exception.

Jason918 approved these changes Jan 20, 2022

View reviewed changes

mattisonchao added 2 commits January 20, 2022 14:11

fixes wrong throw exception.

f9dd4cd

fixes checkstyle

66dec57

mattisonchao mentioned this pull request Jan 20, 2022

[Broker] PersistentTopicsBase has many async method call sync method. #13854

Closed

26 tasks

sijie mentioned this pull request Jan 20, 2022

ISSUE-13854: [Broker] PersistentTopicsBase has many async method call sync method. streamnative/pulsar-archived#3599

Closed

23 tasks

codelipenghui reviewed Jan 21, 2022

View reviewed changes

mattisonchao and others added 2 commits January 21, 2022 21:11

Update pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/imp…

59c8519

…l/PersistentTopicsBase.java Co-authored-by: lipenghui <[email protected]>

Update pulsar-common/src/main/java/org/apache/pulsar/common/util/Futu…

e370c98

…reUtil.java Co-authored-by: lipenghui <[email protected]>

mattisonchao added 3 commits January 25, 2022 10:11

Refactor internalDeletePartitionedTopic method

eed6d3e

Sync method add timeout

6917840

Use lambda instead of for loop.

beabb49

mattisonchao closed this Jan 25, 2022

mattisonchao reopened this Jan 25, 2022

mattisonchao added 3 commits January 25, 2022 11:09

Use unwrapCompletionException instead of unwrapException

8dbbd6c

fix checkstyle

dc8d3ac

Fixes exception handle

77ce9f3

hezhangjian approved these changes Jan 25, 2022

View reviewed changes

Technoboy- reviewed Jan 25, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Show resolved Hide resolved

Fixes missing return and method variable name.

ed37e1e

Technoboy- reviewed Jan 25, 2022

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated Show resolved Hide resolved

fixes method name.

b5a3b17

mattisonchao requested a review from Technoboy- January 25, 2022 12:33

mattisonchao added 2 commits January 25, 2022 20:36

Fixes method name

28cc73e

Fixes checkstyle

0a8c280

mattisonchao closed this Jan 25, 2022

mattisonchao reopened this Jan 25, 2022

mattisonchao requested a review from codelipenghui January 26, 2022 01:17

Technoboy- approved these changes Jan 26, 2022

View reviewed changes

codelipenghui reviewed Jan 26, 2022

View reviewed changes

codelipenghui approved these changes Jan 26, 2022

View reviewed changes

codelipenghui merged commit 0cea8ce into apache:master Jan 26, 2022

codelipenghui removed release/2.8.3 release/2.9.3 labels Jan 27, 2022

michaeljmarshall mentioned this pull request Feb 7, 2022

[Issue 14046] Fixes AsyncResponsedoes not resume when redirected. #14060

Merged

2 tasks

Technoboy- mentioned this pull request Apr 22, 2022

[improve][admin] Put validateTopicOwnershipAsync before validateTopicOperationAsync #15265

Merged

1 task

[Broker] Fix call sync method in async rest api for internalDeletePartitionedTopic #13805

[Broker] Fix call sync method in async rest api for internalDeletePartitionedTopic #13805

Uh oh!

Conversation

mattisonchao commented Jan 18, 2022

Motivation

Modifications

Verifying this change

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

Documentation

Uh oh!

mattisonchao commented Jan 19, 2022

Uh oh!

Uh oh!

mattisonchao commented Jan 19, 2022

Uh oh!

Uh oh!

Uh oh!

mattisonchao commented Jan 20, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattisonchao commented Jan 25, 2022

Uh oh!

mattisonchao commented Jan 26, 2022

Uh oh!

Technoboy- left a comment

Choose a reason for hiding this comment

Uh oh!

codelipenghui Jan 26, 2022

Choose a reason for hiding this comment

Uh oh!

mattisonchao Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[Broker] Fix call sync method in async rest api for `internalDeletePartitionedTopic` #13805

[Broker] Fix call sync method in async rest api for `internalDeletePartitionedTopic` #13805

mattisonchao Jan 26, 2022 •

edited

Loading