Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

remove unnecessary topics cache for Topic Manager #267

Merged
merged 2 commits into from
Dec 28, 2020

Conversation

dockerzhang
Copy link
Contributor

@dockerzhang dockerzhang commented Dec 16, 2020

fixes #266
topics in KafkaTopicManager will cache PersistentTopic by brokerService.getTopic, it's unnecessary because PersistentTopic is cached in brokerService.getTopic. we should remove it to avoid getting a null topic.

@dockerzhang dockerzhang force-pushed the remove-topics-cache branch 2 times, most recently from 0830650 to 441fa83 Compare December 17, 2020 01:15
@jiazhai
Copy link
Contributor

jiazhai commented Dec 17, 2020

@dockerzhang Thanks for the change. Would you please help add the dedicated test cases to test the new code path?
So the code could be protected by the test cases.

@dockerzhang
Copy link
Contributor Author

@jiazhai there is existed KafkaTopicManager UT for this PR.
@BewareMyPower the newest topic cache only serve to remove the producer after the connection close.

@BewareMyPower
Copy link
Collaborator

LGTM. PTAL again @jiazhai

@BewareMyPower BewareMyPower added this to the 2.8.0-M1 milestone Dec 21, 2020
@BewareMyPower BewareMyPower merged commit 0d5ccbe into streamnative:master Dec 28, 2020
BewareMyPower pushed a commit that referenced this pull request Jan 15, 2021
fixes #266
`topics` in `KafkaTopicManager` will cache `PersistentTopic` by `brokerService.getTopic`, it's unnecessary because `PersistentTopic` is cached in `brokerService.getTopic`. we should remove it to avoid getting a `null` topic.
@BewareMyPower BewareMyPower mentioned this pull request Jan 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] redundant topic cache for Topic Manager
3 participants