Skip to content

Ensure the handling of PartitionMetadataRequest is async end-to-end#5307

Merged
merlimat merged 3 commits into
apache:masterfrom
merlimat:get-partitions-async
Oct 8, 2019
Merged

Ensure the handling of PartitionMetadataRequest is async end-to-end#5307
merlimat merged 3 commits into
apache:masterfrom
merlimat:get-partitions-async

Conversation

@merlimat

@merlimat merlimat commented Oct 3, 2019

Copy link
Copy Markdown
Contributor

Motivation

As part of the changes in #4963, the handling of PartitionMetadataRequest is mix of sync and async operation. This is very bad because we're calling that from IO threads which are used for critical publish tasks.

Several internal methods introduced in that commit are async on the surface, though are blocking in practice.

Additionally, the logic for checking that a topic exists is unnecessarily expensive: fetching the list of topics in a namespace and then iterating through the list to compare with the current topic.

Modifications

Refactored the handling of PartitionMetadataRequest to ensure everything is end-to-end async.

@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Oct 3, 2019
@merlimat merlimat added this to the 2.4.2 milestone Oct 3, 2019
@merlimat merlimat self-assigned this Oct 3, 2019
@jiazhai

jiazhai commented Oct 3, 2019

Copy link
Copy Markdown
Member

Thanks for fix this. seems some of related UT failed:
org.apache.pulsar.broker.service.BrokerServiceAutoTopicCreationTest.testAutoTopicCreationDisableIfNonPartitionedTopicAlreadyExist

@sijie

sijie commented Oct 3, 2019

Copy link
Copy Markdown
Member

#4963 is introduced for 2.5.0. so shall this change be marked for 2.5.0?

@merlimat

merlimat commented Oct 3, 2019

Copy link
Copy Markdown
Contributor Author

Yes, I mistakenly tagged for 2.4.2

@merlimat merlimat modified the milestones: 2.4.2, 2.5.0 Oct 3, 2019
@codelipenghui

Copy link
Copy Markdown
Contributor

run integration tests
run java8 tests

@merlimat
merlimat merged commit 71cfe3a into apache:master Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants